Skip to content

Commit d3df6d5

Browse files
committed
fix(format): keep the extra indent of a tighter operator under a looser one
A line inside a group led by an operator of another tier, such as + c, kept the group's indentation when it broke again, so * d lined up under + c and read as (... + c) * d. Such a line now keeps the usual extra indent. A subquery's clauses and a window's PARTITION BY are no longer taken for a group's only operand. VALUES row openers are looked up in a mask instead of a list, so a long VALUES list no longer slows the layout down quadratically. The first commit also stops a long line from breaking before AND, OR or an arithmetic operator that sits between a CASE and its END on that line.
1 parent 296a152 commit d3df6d5

2 files changed

Lines changed: 33 additions & 17 deletions

File tree

‎crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,15 @@ fn given_constructs_sqlquack_converts_to_when_formatting_then_they_are_laid_out(
140140
"SELECT\n CAST(\n CAST(\n (\n total_amount * 100000000\n + adjustment * 100000000\n )\n // 6 AS decimal(38, 0)\n )\n * 0.00000001 AS decimal(18, 8)\n ) AS third\nFROM t",
141141
),
142142
},
143+
test_types::FormatSqlTestCase {
144+
description: "inside a group, a tighter operator broken under a looser one keeps the extra indent that shows it binds first",
145+
sql: "select (first_long_operand_name + second_long_operand_name + third_long_operand_name * fourth_long_operand_name) as v from t",
146+
line_width: Some(40),
147+
dialect: FormatDialect::DuckDB,
148+
expected_outcome: Ok(
149+
"SELECT\n (\n first_long_operand_name\n + second_long_operand_name\n + third_long_operand_name\n * fourth_long_operand_name\n ) AS v\nFROM t",
150+
),
151+
},
143152
test_types::FormatSqlTestCase {
144153
description: "a single-item GROUP BY or ORDER BY and a join's condition without AND or OR stay on their header line",
145154
sql: "select a from t left join u on (t.id = u.id) join v on v.id = t.id and v.k = 1 group by a order by a desc",

‎crates/sqlquack-format/src/format/_helpers/printed_layout.rs‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -457,14 +457,15 @@ impl PrintedSql<'_> {
457457

458458
/// Start the inside of each parenthesised group that spans lines on its own line and close it on its own line, so a group never opens or closes on the line of another; true when a gap changed.
459459
pub(crate) fn separate_multiline_groups(&mut self) -> bool {
460-
let rows: Vec<usize> = (0..self.len())
461-
.filter(|&index| self.is(index, VALUES_KEYWORD))
462-
.flat_map(|values| self.values_rows(values))
463-
.map(|(opener, _)| opener)
464-
.collect();
460+
let mut row_opener = vec![false; self.len()];
461+
for values in (0..self.len()).filter(|&index| self.is(index, VALUES_KEYWORD)) {
462+
for (opener, _) in self.values_rows(values) {
463+
row_opener[opener] = true;
464+
}
465+
}
465466
let mut changed = false;
466-
for opener in 0..self.len() {
467-
if self.tokens[opener] != OPEN_PARENTHESIS || rows.contains(&opener) {
467+
for (opener, &row) in row_opener.iter().enumerate() {
468+
if self.tokens[opener] != OPEN_PARENTHESIS || row {
468469
continue;
469470
}
470471
let Some(closer) = self.partners[opener] else {
@@ -949,25 +950,26 @@ impl PrintedSql<'_> {
949950
parameters.then_some(closer)
950951
}
951952

952-
/// Apply the outermost applicable break: chains, VALUES rows, bracket groups, then commas.
953+
/// Apply the outermost applicable break: chains, VALUES rows, bracket groups, then commas; an operator in a group's only operand takes the group's indentation unless the line starts with another tier's operator, so `+ c` then `* d` cannot read as `(… + c) * d`.
953954
fn break_line(&mut self, line: &Line) -> bool {
954955
let values_row = self.tokens[line.first] == OPEN_PARENTHESIS && self.locked[line.first + 1];
955-
let operator_indent = if self.sole_operand_of_group(line) {
956-
0
957-
} else {
958-
self.indent
959-
};
956+
let sole_operand = self.sole_operand_of_group(line);
957+
let leading_tier = OPERATOR_TIERS
958+
.iter()
959+
.position(|operators| operators.contains(&self.upper(line.first).as_str()));
960+
let indent = self.indent;
960961
!values_row
961962
&& (self.break_before(line, &CHAIN_KEYWORDS, 0)
962-
|| OPERATOR_TIERS
963-
.iter()
964-
.any(|operators| self.break_before(line, operators, operator_indent))
963+
|| OPERATOR_TIERS.iter().enumerate().any(|(tier, operators)| {
964+
let at_group = sole_operand && leading_tier.is_none_or(|lead| lead == tier);
965+
self.break_before(line, operators, if at_group { 0 } else { indent })
966+
})
965967
|| self.break_values_rows(line)
966968
|| self.explode_group(line)
967969
|| self.break_after_commas(line))
968970
}
969971

970-
/// Whether `line` starts or continues with an operator the one operand of a parenthesised group whose inside starts on its own line, so an operator broken onto a new line cannot read as the start of another item.
972+
/// Whether `line` starts or continues with an operator the one operand of a parenthesised group whose inside starts on its own line, so an operator broken onto a new line cannot read as the start of another item. A subquery's clauses and a window's PARTITION BY are never one operand.
971973
fn sole_operand_of_group(&self, line: &Line) -> bool {
972974
let Some(opener) = (0..line.first).rev().find(|&index| {
973975
self.tokens[index] == OPEN_PARENTHESIS
@@ -978,6 +980,11 @@ impl PrintedSql<'_> {
978980
let Some(closer) = self.partners[opener] else {
979981
return false;
980982
};
983+
if self.kinds[opener + 1] == TokenType::Select
984+
|| (opener > 0 && self.is(opener - 1, WINDOW_KEYWORD))
985+
{
986+
return false;
987+
}
981988
let inner = self.depths[opener + 1];
982989
let continues_operand = line.first == opener + 1
983990
|| OPERATOR_TIERS

0 commit comments

Comments
 (0)