diff --git a/crates/pgls_pretty_print/src/nodes/mod.rs b/crates/pgls_pretty_print/src/nodes/mod.rs index dc2f75d61..354ff6f8f 100644 --- a/crates/pgls_pretty_print/src/nodes/mod.rs +++ b/crates/pgls_pretty_print/src/nodes/mod.rs @@ -660,6 +660,34 @@ pub(super) fn emit_condition_on_keyword_line(e: &mut EventEmitter, condition: &N } } +/// Whether a select item without an alias, which would continue on a line of its own, is written as +/// a name or keyword followed by +/// parentheses that hold everything that can break: a function call, `coalesce(...)`, `cast(...)`, +/// a subquery, ... +pub(super) fn breaks_only_inside_its_parentheses(e: &EventEmitter, item: &Node) -> bool { + let Some(NodeEnum::ResTarget(target)) = item.node.as_ref() else { + return false; + }; + if !target.name.is_empty() || !target.indirection.is_empty() { + return false; + } + let Some(value) = target.val.as_deref() else { + return false; + }; + match value.node.as_ref() { + // FILTER, WITHIN GROUP and OVER continue the call on lines of their own. + Some(NodeEnum::FuncCall(call)) => { + call.agg_filter.is_none() && call.over.is_none() && !call.agg_within_group + } + Some(NodeEnum::CoalesceExpr(_) | NodeEnum::MinMaxExpr(_) | NodeEnum::XmlSerialize(_)) => { + true + } + Some(NodeEnum::TypeCast(_)) => matches!(e.config().cast_style, crate::CastStyle::Cast), + Some(NodeEnum::SubLink(_)) => breaks_only_inside_a_subquery(value), + _ => false, + } +} + fn breaks_only_inside_a_subquery(node: &Node) -> bool { use pgls_query::protobuf::{AExprKind, BoolExprType, SubLinkType}; diff --git a/crates/pgls_pretty_print/src/nodes/select_stmt.rs b/crates/pgls_pretty_print/src/nodes/select_stmt.rs index f4bb92ddf..345de1faf 100644 --- a/crates/pgls_pretty_print/src/nodes/select_stmt.rs +++ b/crates/pgls_pretty_print/src/nodes/select_stmt.rs @@ -200,18 +200,30 @@ fn emit_select_stmt_impl(e: &mut EventEmitter, n: &SelectStmt, with_semicolon: b } if !n.target_list.is_empty() { - e.indent_start(); - // Compact keeps a lone item on the SELECT line, `select 1`; a list of several still - // puts one item per line. - match e.config().clause_body_style { - ClauseBodyStyle::Compact if n.target_list.len() == 1 => e.space(), - _ => super::emit_layout_break(e), + // puts one item per line. The indent is for the item's continuation lines, which an + // item that only breaks inside its parentheses, `coalesce(...)`, doesn't have: indented, + // its arguments would sit two levels in and its `)` one level in. + let lone_compact_item = + matches!(e.config().clause_body_style, ClauseBodyStyle::Compact) + && n.target_list.len() == 1; + let indent = !(lone_compact_item + && super::breaks_only_inside_its_parentheses(e, &n.target_list[0])); + + if indent { + e.indent_start(); + } + if lone_compact_item { + e.space(); + } else { + super::emit_layout_break(e); } emit_comma_separated_list_with_layout_break(e, &n.target_list, super::emit_node); - e.indent_end(); + if indent { + e.indent_end(); + } } // Emit INTO clause if present (SELECT ... INTO table_name) 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 8f66e3ae5..d057c4274 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 @@ -39,10 +39,10 @@ select case from users; select json_build_object( - 'identifier', users.id, - 'display_name', users.name, - 'email_address', users.email - ) + 'identifier', users.id, + 'display_name', users.name, + 'email_address', users.email +) from users; select first_really_long_column_name_here + second_really_long_column_name_here + third_long_column 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 04cbb3ede..4d0036712 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 @@ -42,10 +42,10 @@ select case from users; select json_build_object( - 'identifier', users.id, - 'display_name', users.name, - 'email_address', users.email - ) + 'identifier', users.id, + 'display_name', users.name, + 'email_address', users.email +) from users; select first_really_long_column_name_here + 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 d6caf1f25..503511ecd 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 @@ -55,10 +55,10 @@ select case from users; select json_build_object( - 'identifier', users.id, - 'display_name', users.name, - 'email_address', users.email - ) + 'identifier', users.id, + 'display_name', users.name, + 'email_address', users.email +) from users; select first_really_long_column_name_here + second_really_long_column_name_here + third_long_column 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 236cba85c..261b8c7e2 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 @@ -55,10 +55,10 @@ select case from users; select json_build_object( - 'identifier', users.id, - 'display_name', users.name, - 'email_address', users.email - ) + 'identifier', users.id, + 'display_name', users.name, + 'email_address', users.email +) from users; select first_really_long_column_name_here + 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 4774352f4..0c27896a7 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 @@ -39,10 +39,10 @@ select case from users; select json_build_object( - 'identifier', users.id - , 'display_name', users.name - , 'email_address', users.email - ) + 'identifier', users.id + , 'display_name', users.name + , 'email_address', users.email +) from users; select first_really_long_column_name_here + second_really_long_column_name_here + third_long_column 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 c0bf2ce57..444b57e7c 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 @@ -42,10 +42,10 @@ select case from users; select json_build_object( - 'identifier', users.id - , 'display_name', users.name - , 'email_address', users.email - ) + 'identifier', users.id + , 'display_name', users.name + , 'email_address', users.email +) from users; select first_really_long_column_name_here + 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 6332e0814..2dea40416 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 @@ -37,14 +37,14 @@ 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 max(x) + from y + where y.id = t.id + and y.kind = 'some kind of value' + ), + 0 + ) ); select * 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 6332e0814..2dea40416 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 @@ -37,14 +37,14 @@ 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 max(x) + from y + where y.id = t.id + and y.kind = 'some kind of value' + ), + 0 + ) ); select * diff --git a/crates/pgls_pretty_print/tests/subquery_layout.rs b/crates/pgls_pretty_print/tests/subquery_layout.rs index 24bc4b274..232e253e6 100644 --- a/crates/pgls_pretty_print/tests/subquery_layout.rs +++ b/crates/pgls_pretty_print/tests/subquery_layout.rs @@ -119,3 +119,29 @@ fn a_subquery_opened_on_the_on_line_closes_under_on() { assert!(output.contains("\n on u.id = (\n select\n"), "{output}"); assert!(output.ends_with("\n );"), "{output}"); } + +#[test] +fn a_lone_compact_select_item_closes_under_select() { + let config = FormatConfig { + clause_body_style: ClauseBodyStyle::Compact, + line_width: 60, + ..Default::default() + }; + assert_eq!( + format_idempotently( + "select coalesce(some_long_column_name, another_long_column_name, 0) from t", + &config, + ), + "select coalesce(\n some_long_column_name,\n another_long_column_name,\n 0\n)\nfrom t;" + ); + + // An item with continuation lines of its own keeps the indent for them. + let output = format_idempotently( + "select some_long_column_name + another_long_column_name + a_third_long_column from t", + &config, + ); + assert_eq!( + output, + "select some_long_column_name + another_long_column_name +\n a_third_long_column\nfrom t;" + ); +}