diff --git a/crates/pgls_pretty_print/src/nodes/join_expr.rs b/crates/pgls_pretty_print/src/nodes/join_expr.rs index 7492ee28c..1f8eb9a4d 100644 --- a/crates/pgls_pretty_print/src/nodes/join_expr.rs +++ b/crates/pgls_pretty_print/src/nodes/join_expr.rs @@ -123,9 +123,7 @@ pub(super) fn emit_join_expr(e: &mut EventEmitter, n: &JoinExpr) { e.line(LineType::SoftOrSpace); e.token(TokenKind::ON_KW); e.space(); - e.indent_start(); - super::emit_node(quals, e); - e.indent_end(); + super::emit_condition_on_keyword_line(e, quals); } else if matches!(jointype, JoinType::JoinInner) && !n.is_natural { // For INNER JOIN without qualifications (converted from CROSS JOIN), add ON TRUE // This is semantically equivalent to CROSS JOIN diff --git a/crates/pgls_pretty_print/src/nodes/mod.rs b/crates/pgls_pretty_print/src/nodes/mod.rs index 72e986b2b..996938a17 100644 --- a/crates/pgls_pretty_print/src/nodes/mod.rs +++ b/crates/pgls_pretty_print/src/nodes/mod.rs @@ -609,13 +609,69 @@ pub(super) fn emit_clause_condition(e: &mut EventEmitter, clause: &Node) { // Compact keeps the condition on the keyword line. The indent stays in both cases so that the // condition's own continuation lines, the second AND of a chain for instance, sit under it. match e.config().clause_body_style { - ClauseBodyStyle::Compact => e.space(), - ClauseBodyStyle::Break => e.line(LineType::SoftOrSpace), + ClauseBodyStyle::Compact => { + e.space(); + emit_condition_on_keyword_line(e, clause); + } + ClauseBodyStyle::Break => { + e.line(LineType::SoftOrSpace); + e.indent_start(); + emit_node(clause, e); + e.indent_end(); + } } +} - e.indent_start(); - emit_node(clause, e); - e.indent_end(); +/// Emits a condition that starts on its keyword's line, `where` or `on`, indenting its +/// continuation lines. +/// +/// A condition that only breaks inside the parentheses of a subquery, `exists (...)` or +/// `a = (...)`, has no continuation lines of its own. Indenting it would put the subquery two +/// levels in and the closing parenthesis one level in, so it keeps the keyword's indent and closes +/// under the keyword. +pub(super) fn emit_condition_on_keyword_line(e: &mut EventEmitter, condition: &Node) { + if breaks_only_inside_a_subquery(condition) { + emit_node(condition, e); + } else { + e.indent_start(); + emit_node(condition, e); + e.indent_end(); + } +} + +fn breaks_only_inside_a_subquery(node: &Node) -> bool { + use pgls_query::protobuf::{AExprKind, BoolExprType, SubLinkType}; + + match node.node.as_ref() { + Some(NodeEnum::SubLink(sub_link)) => { + sub_link.sub_link_type != SubLinkType::MultiexprSublink as i32 + } + Some(NodeEnum::AExpr(expr)) => { + expr.kind() == AExprKind::AexprOp + && expr.lexpr.as_deref().is_some_and(is_simple_operand) + && expr.rexpr.as_deref().is_some_and(|rexpr| { + matches!( + rexpr.node.as_ref(), + Some(NodeEnum::SubLink(sub_link)) + if sub_link.sub_link_type == SubLinkType::ExprSublink as i32 + || sub_link.sub_link_type == SubLinkType::ArraySublink as i32 + ) + }) + } + Some(NodeEnum::BoolExpr(expr)) => { + expr.boolop() == BoolExprType::NotExpr + && matches!(expr.args.as_slice(), [arg] if breaks_only_inside_a_subquery(arg)) + } + _ => false, + } +} + +/// A column reference, parameter or constant, which never breaks. +fn is_simple_operand(node: &Node) -> bool { + matches!( + node.node.as_ref(), + Some(NodeEnum::ColumnRef(_) | NodeEnum::ParamRef(_) | NodeEnum::AConst(_)) + ) } /// Emits a condition wrapped in parentheses, such as the `USING (...)` of a policy. diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_100.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_100.snap index d5de5d413..8f66e3ae5 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_100.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_100.snap @@ -26,10 +26,10 @@ select id into temporary table active_user_ids from users where active; select * from users where exists ( - select 1 - from memberships - where memberships.user_id = users.id and memberships.organization_id = 42 - ); + select 1 + from memberships + where memberships.user_id = users.id and memberships.organization_id = 42 +); select case when users.status = 'active' then 'currently an active member' diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_80.snap index 43fbe17c2..04cbb3ede 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_80.snap @@ -29,10 +29,10 @@ select id into temporary table active_user_ids from users where active; select * from users where exists ( - select 1 - from memberships - where memberships.user_id = users.id and memberships.organization_id = 42 - ); + select 1 + from memberships + where memberships.user_id = users.id and memberships.organization_id = 42 +); select case when users.status = 'active' then 'currently an active member' diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_expanded_100.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_expanded_100.snap index 5367b6777..d6caf1f25 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_expanded_100.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_expanded_100.snap @@ -39,11 +39,11 @@ where active; select * from users where exists ( - select 1 - from memberships - where memberships.user_id = users.id and - memberships.organization_id = 42 - ); + select 1 + from memberships + where memberships.user_id = users.id and + memberships.organization_id = 42 +); select case when users.status = 'active' diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_expanded_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_expanded_80.snap index 579b565cd..236cba85c 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_expanded_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_expanded_80.snap @@ -39,11 +39,11 @@ where active; select * from users where exists ( - select 1 - from memberships - where memberships.user_id = users.id and - memberships.organization_id = 42 - ); + select 1 + from memberships + where memberships.user_id = users.id and + memberships.organization_id = 42 +); select case when users.status = 'active' diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_leading_100.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_leading_100.snap index 278afe35f..4774352f4 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_leading_100.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_leading_100.snap @@ -26,10 +26,10 @@ select id into temporary table active_user_ids from users where active; select * from users where exists ( - select 1 - from memberships - where memberships.user_id = users.id and memberships.organization_id = 42 - ); + select 1 + from memberships + where memberships.user_id = users.id and memberships.organization_id = 42 +); select case when users.status = 'active' then 'currently an active member' diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_leading_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_leading_80.snap index 43f922d77..c0bf2ce57 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_leading_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__compact_select_list_leading_80.snap @@ -29,10 +29,10 @@ select id into temporary table active_user_ids from users where active; select * from users where exists ( - select 1 - from memberships - where memberships.user_id = users.id and memberships.organization_id = 42 - ); + select 1 + from memberships + where memberships.user_id = users.id and memberships.organization_id = 42 +); select case when users.status = 'active' then 'currently an active member' diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__predicate_100.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__predicate_100.snap index 47cec56d0..8f4d135a8 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__predicate_100.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__predicate_100.snap @@ -71,16 +71,16 @@ from left outer join pred_tab as t2 on exists ( - select - 1 - from - pred_tab as t3, - pred_tab as t4, - pred_tab as t5, - pred_tab as t6 - where - t1.a = t3.a and t6.a is not null - ); + select + 1 + from + pred_tab as t3, + pred_tab as t4, + pred_tab as t5, + pred_tab as t6 + where + t1.a = t3.a and t6.a is not null + ); select * @@ -89,16 +89,16 @@ from left outer join pred_tab as t2 on exists ( - select - 1 - from - pred_tab as t3, - pred_tab as t4, - pred_tab as t5, - pred_tab as t6 - where - t1.a = t3.a and t6.a is null - ); + select + 1 + from + pred_tab as t3, + pred_tab as t4, + pred_tab as t5, + pred_tab as t6 + where + t1.a = t3.a and t6.a is null + ); drop table pred_tab; diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__predicate_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__predicate_80.snap index 092589b35..46a9fb31e 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__predicate_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__predicate_80.snap @@ -77,16 +77,16 @@ from left outer join pred_tab as t2 on exists ( - select - 1 - from - pred_tab as t3, - pred_tab as t4, - pred_tab as t5, - pred_tab as t6 - where - t1.a = t3.a and t6.a is not null - ); + select + 1 + from + pred_tab as t3, + pred_tab as t4, + pred_tab as t5, + pred_tab as t6 + where + t1.a = t3.a and t6.a is not null + ); select * @@ -95,16 +95,16 @@ from left outer join pred_tab as t2 on exists ( - select - 1 - from - pred_tab as t3, - pred_tab as t4, - pred_tab as t5, - pred_tab as t6 - where - t1.a = t3.a and t6.a is null - ); + select + 1 + from + pred_tab as t3, + pred_tab as t4, + pred_tab as t5, + pred_tab as t6 + where + t1.a = t3.a and t6.a is null + ); drop table pred_tab; diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__scalar_subquery_expanded_100.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__scalar_subquery_expanded_100.snap index c7853c85c..6332e0814 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__scalar_subquery_expanded_100.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__scalar_subquery_expanded_100.snap @@ -18,34 +18,34 @@ using ( select * from documents as d where d.owner_id = ( - select m.user_id - from memberships as m - where m.organization_id = d.organization_id - limit 1 - ); + select m.user_id + from memberships as m + where m.organization_id = d.organization_id + limit 1 +); select * from documents where tags = array( - select tag - from document_tags - where document_tags.document_id = documents.id - order by tag - ); + select tag + from document_tags + where document_tags.document_id = documents.id + order by tag +); select * from t where a = ( - select coalesce( - ( - select max(x) - from y - where y.id = t.id - and y.kind = 'some kind of value' - ), - 0 - ) - ); + select coalesce( + ( + select max(x) + from y + where y.id = t.id + and y.kind = 'some kind of value' + ), + 0 + ) +); select * from t diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__scalar_subquery_expanded_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__scalar_subquery_expanded_80.snap index c7853c85c..6332e0814 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__scalar_subquery_expanded_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__scalar_subquery_expanded_80.snap @@ -18,34 +18,34 @@ using ( select * from documents as d where d.owner_id = ( - select m.user_id - from memberships as m - where m.organization_id = d.organization_id - limit 1 - ); + select m.user_id + from memberships as m + where m.organization_id = d.organization_id + limit 1 +); select * from documents where tags = array( - select tag - from document_tags - where document_tags.document_id = documents.id - order by tag - ); + select tag + from document_tags + where document_tags.document_id = documents.id + order by tag +); select * from t where a = ( - select coalesce( - ( - select max(x) - from y - where y.id = t.id - and y.kind = 'some kind of value' - ), - 0 - ) - ); + select coalesce( + ( + select max(x) + from y + where y.id = t.id + and y.kind = 'some kind of value' + ), + 0 + ) +); select * from t diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__sublink_bool_groups_expanded_100.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__sublink_bool_groups_expanded_100.snap index 7cc2afc1a..53020eaac 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__sublink_bool_groups_expanded_100.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__sublink_bool_groups_expanded_100.snap @@ -36,26 +36,26 @@ using ( select * from public.permissions where exists ( - select 1 - from public.permission_set_permissions as rp - where rp.permission_name = permissions.name - ); + select 1 + from public.permission_set_permissions as rp + where rp.permission_name = permissions.name +); select * from public.permissions where name = any ( - select permission_name - from public.permission_set_permissions - where position_uuid is not null - ); + select permission_name + from public.permission_set_permissions + where position_uuid is not null +); select * from public.permissions where id > all ( - select permission_id - from public.permission_set_permissions - where position_uuid is not null - ); + select permission_id + from public.permission_set_permissions + where position_uuid is not null +); select * from public.permissions diff --git a/crates/pgls_pretty_print/tests/snapshots/multi/tests__sublink_bool_groups_expanded_80.snap b/crates/pgls_pretty_print/tests/snapshots/multi/tests__sublink_bool_groups_expanded_80.snap index 7cc2afc1a..53020eaac 100644 --- a/crates/pgls_pretty_print/tests/snapshots/multi/tests__sublink_bool_groups_expanded_80.snap +++ b/crates/pgls_pretty_print/tests/snapshots/multi/tests__sublink_bool_groups_expanded_80.snap @@ -36,26 +36,26 @@ using ( select * from public.permissions where exists ( - select 1 - from public.permission_set_permissions as rp - where rp.permission_name = permissions.name - ); + select 1 + from public.permission_set_permissions as rp + where rp.permission_name = permissions.name +); select * from public.permissions where name = any ( - select permission_name - from public.permission_set_permissions - where position_uuid is not null - ); + select permission_name + from public.permission_set_permissions + where position_uuid is not null +); select * from public.permissions where id > all ( - select permission_id - from public.permission_set_permissions - where position_uuid is not null - ); + select permission_id + from public.permission_set_permissions + where position_uuid is not null +); select * from public.permissions diff --git a/crates/pgls_pretty_print/tests/subquery_layout.rs b/crates/pgls_pretty_print/tests/subquery_layout.rs index 9cf960aff..24bc4b274 100644 --- a/crates/pgls_pretty_print/tests/subquery_layout.rs +++ b/crates/pgls_pretty_print/tests/subquery_layout.rs @@ -74,3 +74,48 @@ fn nested_subquery_inside_select_list_only_subquery_expands() { "{output}" ); } + +#[test] +fn a_subquery_opened_on_the_where_line_closes_under_where() { + let config = FormatConfig { + layout: Layout::Expanded, + clause_body_style: ClauseBodyStyle::Compact, + ..Default::default() + }; + assert_eq!( + format_idempotently( + "select * from t where exists (select 1 from u where u.id = t.id)", + &config, + ), + "select *\nfrom t\nwhere exists (\n select 1\n from u\n where u.id = t.id\n);" + ); + assert_eq!( + format_idempotently( + "select * from t where t.a = (select max(x) from u where u.id = t.id)", + &config, + ), + "select *\nfrom t\nwhere t.a = (\n select max(x)\n from u\n where u.id = t.id\n);" + ); + + // A chain still indents its continuation lines, and a subquery in it closes on its own line. + let output = format_idempotently( + "select * from t where t.a = 1 and exists (select 1 from u where u.id = t.id)", + &config, + ); + assert!(output.contains("\n exists (\n select 1\n"), "{output}"); + assert!(output.ends_with("\n );"), "{output}"); +} + +#[test] +fn a_subquery_opened_on_the_on_line_closes_under_on() { + let config = FormatConfig { + line_width: 60, + ..Default::default() + }; + let output = format_idempotently( + "select * from t inner join u on u.id = (select max(id) from versions where versions.name = t.name)", + &config, + ); + assert!(output.contains("\n on u.id = (\n select\n"), "{output}"); + assert!(output.ends_with("\n );"), "{output}"); +}