From 5848906edb6dc5d66662cc94944d35abe35ab0f0 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Tue, 13 May 2025 22:54:37 +0200 Subject: [PATCH 1/8] fix pretty-printing of multi-line literal strings this also improves performance and simplifies pretty-printing --- src/display_utils.rs | 45 +++++++++++++++---------------------------- tests/pretty_print.rs | 14 ++++++++++++++ 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/display_utils.rs b/src/display_utils.rs index e7e1272fb..39ae30f3b 100644 --- a/src/display_utils.rs +++ b/src/display_utils.rs @@ -31,13 +31,10 @@ where T: Write, { fn write_str(&mut self, s: &str) -> fmt::Result { - let mut first = true; - for line in s.split('\n') { - if !first { - write!(self.0, "\n{INDENT}")?; - } - self.0.write_str(line)?; - first = false; + self.0.write_str(s)?; + // Our NewLine and SpaceOrNewline utils always print individual newlines as a single-character string. + if s == "\n" { + self.0.write_str(INDENT)?; } Ok(()) } @@ -98,36 +95,24 @@ pub(crate) fn indented_list(f: &mut fmt::Formatter, slice: &[T] mod tests { use super::*; - struct DisplayCharByChar(T); + #[test] + fn test_indent() { + struct TwoLines; - impl Display for DisplayCharByChar { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for c in self.0.to_string().chars() { - write!(f, "{}", c)?; + impl Display for TwoLines { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("line 1")?; + SpaceOrNewline.fmt(f)?; + f.write_str("line 2") } - Ok(()) } - } - #[test] - fn test_indent() { - let original = "line 1\nline 2"; - let indent = Indent(original); + let indent = Indent(TwoLines); assert_eq!( indent.to_string(), - original, + TwoLines.to_string(), "Only the alternate form should be indented" ); - let expected = " line 1\n line 2"; - assert_eq!(format!("{:#}", indent), expected); - let display_char_by_char = DisplayCharByChar(original); - assert_eq!(format!("{:#}", Indent(display_char_by_char)), expected); - } - - #[test] - fn test_space_or_newline() { - let space_or_newline = SpaceOrNewline; - assert_eq!(format!("{}", space_or_newline), " "); - assert_eq!(format!("{:#}", space_or_newline), "\n"); + assert_eq!(format!("{:#}", indent), " line 1\n line 2"); } } diff --git a/tests/pretty_print.rs b/tests/pretty_print.rs index 1eb8ca41c..361ef0258 100644 --- a/tests/pretty_print.rs +++ b/tests/pretty_print.rs @@ -155,3 +155,17 @@ FROM "#.trim() ); } + +#[test] +fn test_pretty_print_multiline_string() { + assert_eq!( + prettify("SELECT 'multiline\nstring' AS str"), + r#" +SELECT + 'multiline +string' AS str +"# + .trim(), + "A literal string with a newline should be kept as is. The contents of the string should not be indented." + ); +} From 6fd27d5418b18d0bac397e38ad47893f10b4005f Mon Sep 17 00:00:00 2001 From: lovasoa Date: Tue, 13 May 2025 22:59:13 +0200 Subject: [PATCH 2/8] readme example for pretty printing --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 6acfbcef7..666be17c0 100644 --- a/README.md +++ b/README.md @@ -89,10 +89,14 @@ keywords, the following should hold true for all SQL: ```rust // Parse SQL +let sql = "SELECT 'hello'"; let ast = Parser::parse_sql(&GenericDialect, sql).unwrap(); // The original SQL text can be generated from the AST assert_eq!(ast[0].to_string(), sql); + +// The SQL can also be pretty-printed with newlines and indentation +assert_eq!(format!("{:#}", ast[0]), "SELECT\n 'hello'"); ``` There are still some cases in this crate where different SQL with seemingly From 83bc1799e92828949e8904c3b5e43ab69ce78ce6 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Tue, 13 May 2025 23:07:05 +0200 Subject: [PATCH 3/8] Add pretty-printing tests for various SQL statements see https://github.com/apache/datafusion-sqlparser-rs/issues/1850 all the tests are skipped at the moment, we will enable them as we implement more pretty printing --- tests/pretty_print.rs | 213 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 213 insertions(+) diff --git a/tests/pretty_print.rs b/tests/pretty_print.rs index 361ef0258..ea6e351c3 100644 --- a/tests/pretty_print.rs +++ b/tests/pretty_print.rs @@ -169,3 +169,216 @@ string' AS str "A literal string with a newline should be kept as is. The contents of the string should not be indented." ); } + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_insert_values() { + assert_eq!( + prettify("INSERT INTO my_table (a, b, c) VALUES (1, 2, 3), (4, 5, 6)"), + r#" +INSERT INTO my_table (a, b, c) +VALUES + (1, 2, 3), + (4, 5, 6) +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_insert_select() { + assert_eq!( + prettify("INSERT INTO my_table (a, b) SELECT x, y FROM source_table"), + r#" +INSERT INTO my_table (a, b) +SELECT + x, + y +FROM + source_table +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_update() { + assert_eq!( + prettify("UPDATE my_table SET a = 1, b = 2 WHERE x > 0"), + r#" +UPDATE my_table +SET + a = 1, + b = 2 +WHERE + x > 0 +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_delete() { + assert_eq!( + prettify("DELETE FROM my_table WHERE x > 0"), + r#" +DELETE FROM my_table +WHERE + x > 0 +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_create_table() { + assert_eq!( + prettify("CREATE TABLE my_table (id INT PRIMARY KEY, name VARCHAR(255) NOT NULL, CONSTRAINT fk_other FOREIGN KEY (id) REFERENCES other_table(id))"), + r#" +CREATE TABLE my_table ( + id INT PRIMARY KEY, + name VARCHAR(255) NOT NULL, + CONSTRAINT fk_other FOREIGN KEY (id) REFERENCES other_table(id) +) +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_create_view() { + assert_eq!( + prettify("CREATE VIEW my_view AS SELECT a, b FROM my_table WHERE x > 0"), + r#" +CREATE VIEW my_view AS +SELECT + a, + b +FROM + my_table +WHERE + x > 0 +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_create_function() { + assert_eq!( + prettify("CREATE FUNCTION my_func() RETURNS INT BEGIN SELECT COUNT(*) INTO @count FROM my_table; RETURN @count; END"), + r#" +CREATE FUNCTION my_func() RETURNS INT +BEGIN + SELECT COUNT(*) INTO @count FROM my_table; + RETURN @count; +END +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_json_table() { + assert_eq!( + prettify("SELECT * FROM JSON_TABLE(@json, '$[*]' COLUMNS (id INT PATH '$.id', name VARCHAR(255) PATH '$.name')) AS jt"), + r#" +SELECT + * +FROM + JSON_TABLE( + @json, + '$[*]' COLUMNS ( + id INT PATH '$.id', + name VARCHAR(255) PATH '$.name' + ) + ) AS jt +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_transaction_blocks() { + assert_eq!( + prettify("BEGIN; UPDATE my_table SET x = 1; COMMIT;"), + r#" +BEGIN; +UPDATE my_table SET x = 1; +COMMIT; +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_control_flow() { + assert_eq!( + prettify("IF x > 0 THEN SELECT 'positive'; ELSE SELECT 'negative'; END IF;"), + r#" +IF x > 0 THEN + SELECT 'positive'; +ELSE + SELECT 'negative'; +END IF; +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_merge() { + assert_eq!( + prettify("MERGE INTO target_table t USING source_table s ON t.id = s.id WHEN MATCHED THEN UPDATE SET t.value = s.value WHEN NOT MATCHED THEN INSERT (id, value) VALUES (s.id, s.value)"), + r#" +MERGE INTO target_table t +USING source_table s ON t.id = s.id +WHEN MATCHED THEN + UPDATE SET t.value = s.value +WHEN NOT MATCHED THEN + INSERT (id, value) VALUES (s.id, s.value) +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_create_index() { + assert_eq!( + prettify("CREATE INDEX idx_name ON my_table (column1, column2)"), + r#" +CREATE INDEX idx_name +ON my_table (column1, column2) +"# + .trim() + ); +} + +#[test] +#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] +fn test_pretty_print_explain() { + assert_eq!( + prettify("EXPLAIN ANALYZE SELECT * FROM my_table WHERE x > 0"), + r#" +EXPLAIN ANALYZE +SELECT + * +FROM + my_table +WHERE + x > 0 +"# + .trim() + ); +} From 15dc4406307d1fbbf0d7eb74ac09d2283dc7a1ff Mon Sep 17 00:00:00 2001 From: lovasoa Date: Tue, 13 May 2025 23:16:16 +0200 Subject: [PATCH 4/8] pretty print update --- src/ast/mod.rs | 30 +++++++++++++++++++++--------- src/display_utils.rs | 2 +- tests/pretty_print.rs | 6 ++++-- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 79b0e0d52..f8894259c 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -41,7 +41,7 @@ use serde::{Deserialize, Serialize}; use sqlparser_derive::{Visit, VisitMut}; use crate::{ - display_utils::SpaceOrNewline, + display_utils::{indented_list, SpaceOrNewline}, tokenizer::{Span, Token}, }; use crate::{ @@ -4606,25 +4606,37 @@ impl fmt::Display for Statement { returning, or, } => { - write!(f, "UPDATE ")?; + f.write_str("UPDATE ")?; if let Some(or) = or { - write!(f, "{or} ")?; + or.fmt(f)?; + f.write_str(" ")?; } - write!(f, "{table}")?; + table.fmt(f)?; if let Some(UpdateTableFromKind::BeforeSet(from)) = from { - write!(f, " FROM {}", display_comma_separated(from))?; + SpaceOrNewline.fmt(f)?; + f.write_str("FROM")?; + indented_list(f, from)?; } if !assignments.is_empty() { - write!(f, " SET {}", display_comma_separated(assignments))?; + SpaceOrNewline.fmt(f)?; + f.write_str("SET")?; + indented_list(f, assignments)?; } if let Some(UpdateTableFromKind::AfterSet(from)) = from { - write!(f, " FROM {}", display_comma_separated(from))?; + SpaceOrNewline.fmt(f)?; + f.write_str("FROM")?; + indented_list(f, from)?; } if let Some(selection) = selection { - write!(f, " WHERE {selection}")?; + SpaceOrNewline.fmt(f)?; + f.write_str("WHERE")?; + SpaceOrNewline.fmt(f)?; + Indent(selection).fmt(f)?; } if let Some(returning) = returning { - write!(f, " RETURNING {}", display_comma_separated(returning))?; + SpaceOrNewline.fmt(f)?; + f.write_str("RETURNING")?; + indented_list(f, returning)?; } Ok(()) } diff --git a/src/display_utils.rs b/src/display_utils.rs index 39ae30f3b..28d8e415b 100644 --- a/src/display_utils.rs +++ b/src/display_utils.rs @@ -68,7 +68,7 @@ impl Display for SpaceOrNewline { /// A value that displays a comma-separated list of values. /// When pretty-printed (using {:#}), it displays each value on a new line. -pub struct DisplayCommaSeparated<'a, T: fmt::Display>(&'a [T]); +pub(crate) struct DisplayCommaSeparated<'a, T: fmt::Display>(&'a [T]); impl fmt::Display for DisplayCommaSeparated<'_, T> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { diff --git a/tests/pretty_print.rs b/tests/pretty_print.rs index ea6e351c3..77042dc48 100644 --- a/tests/pretty_print.rs +++ b/tests/pretty_print.rs @@ -203,10 +203,9 @@ FROM } #[test] -#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] fn test_pretty_print_update() { assert_eq!( - prettify("UPDATE my_table SET a = 1, b = 2 WHERE x > 0"), + prettify("UPDATE my_table SET a = 1, b = 2 WHERE x > 0 RETURNING id, name"), r#" UPDATE my_table SET @@ -214,6 +213,9 @@ SET b = 2 WHERE x > 0 +RETURNING + id, + name "# .trim() ); From a6b0ce52271a4b24c0c0756a941efe391eecdeb7 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Tue, 13 May 2025 23:33:03 +0200 Subject: [PATCH 5/8] pretty-print DELETE --- src/ast/dml.rs | 33 ++++++++++++++++++++++++--------- src/ast/mod.rs | 4 ++-- tests/pretty_print.rs | 19 ++++++++++++++++--- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/ast/dml.rs b/src/ast/dml.rs index 7ed17be9b..e28e5d1d4 100644 --- a/src/ast/dml.rs +++ b/src/ast/dml.rs @@ -29,6 +29,8 @@ use serde::{Deserialize, Serialize}; #[cfg(feature = "visitor")] use sqlparser_derive::{Visit, VisitMut}; +use crate::display_utils::{indented_list, Indent, SpaceOrNewline}; + pub use super::ddl::{ColumnDef, TableConstraint}; use super::{ @@ -649,32 +651,45 @@ pub struct Delete { impl Display for Delete { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "DELETE ")?; + f.write_str("DELETE")?; if !self.tables.is_empty() { - write!(f, "{} ", display_comma_separated(&self.tables))?; + indented_list(f, &self.tables)?; } match &self.from { FromTable::WithFromKeyword(from) => { - write!(f, "FROM {}", display_comma_separated(from))?; + f.write_str(" FROM")?; + indented_list(f, from)?; } FromTable::WithoutKeyword(from) => { - write!(f, "{}", display_comma_separated(from))?; + indented_list(f, from)?; } } if let Some(using) = &self.using { - write!(f, " USING {}", display_comma_separated(using))?; + SpaceOrNewline.fmt(f)?; + f.write_str("USING")?; + indented_list(f, using)?; } if let Some(selection) = &self.selection { - write!(f, " WHERE {selection}")?; + SpaceOrNewline.fmt(f)?; + f.write_str("WHERE")?; + SpaceOrNewline.fmt(f)?; + Indent(selection).fmt(f)?; } if let Some(returning) = &self.returning { - write!(f, " RETURNING {}", display_comma_separated(returning))?; + SpaceOrNewline.fmt(f)?; + f.write_str("RETURNING")?; + indented_list(f, returning)?; } if !self.order_by.is_empty() { - write!(f, " ORDER BY {}", display_comma_separated(&self.order_by))?; + SpaceOrNewline.fmt(f)?; + f.write_str("ORDER BY")?; + indented_list(f, &self.order_by)?; } if let Some(limit) = &self.limit { - write!(f, " LIMIT {limit}")?; + SpaceOrNewline.fmt(f)?; + f.write_str("LIMIT")?; + SpaceOrNewline.fmt(f)?; + Indent(limit).fmt(f)?; } Ok(()) } diff --git a/src/ast/mod.rs b/src/ast/mod.rs index f8894259c..a572e9d25 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -4640,8 +4640,8 @@ impl fmt::Display for Statement { } Ok(()) } - Statement::Delete(delete) => write!(f, "{delete}"), - Statement::Open(open) => write!(f, "{open}"), + Statement::Delete(delete) => delete.fmt(f), + Statement::Open(open) => open.fmt(f), Statement::Close { cursor } => { write!(f, "CLOSE {cursor}")?; diff --git a/tests/pretty_print.rs b/tests/pretty_print.rs index 77042dc48..9e68417d7 100644 --- a/tests/pretty_print.rs +++ b/tests/pretty_print.rs @@ -222,14 +222,27 @@ RETURNING } #[test] -#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] fn test_pretty_print_delete() { assert_eq!( - prettify("DELETE FROM my_table WHERE x > 0"), + prettify("DELETE FROM my_table WHERE x > 0 RETURNING id, name"), r#" -DELETE FROM my_table +DELETE FROM + my_table WHERE x > 0 +RETURNING + id, + name +"# + .trim() + ); + + assert_eq!( + prettify("DELETE table1, table2"), + r#" +DELETE + table1, + table2 "# .trim() ); From 2a0f1813f8f82f329f6d70b10f6f787a5761e5ab Mon Sep 17 00:00:00 2001 From: lovasoa Date: Wed, 14 May 2025 00:13:37 +0200 Subject: [PATCH 6/8] pretty-printing for INSERT statements and VALUES clause - Refactor DisplayCommaSeparated to accept any iterable type. - Update indented_list to work with iterators. - Improve formatting in the Insert display implementation --- src/ast/dml.rs | 24 +++++++++++++++--------- src/ast/mod.rs | 2 +- src/ast/query.rs | 26 +++++++++++++++++++------- src/display_utils.rs | 18 +++++++++++++----- tests/pretty_print.rs | 6 +++--- 5 files changed, 51 insertions(+), 25 deletions(-) diff --git a/src/ast/dml.rs b/src/ast/dml.rs index e28e5d1d4..e4081a637 100644 --- a/src/ast/dml.rs +++ b/src/ast/dml.rs @@ -581,28 +581,32 @@ impl Display for Insert { )?; } if !self.columns.is_empty() { - write!(f, "({}) ", display_comma_separated(&self.columns))?; + write!(f, "({})", display_comma_separated(&self.columns))?; + SpaceOrNewline.fmt(f)?; } if let Some(ref parts) = self.partitioned { if !parts.is_empty() { - write!(f, "PARTITION ({}) ", display_comma_separated(parts))?; + write!(f, "PARTITION ({})", display_comma_separated(parts))?; + SpaceOrNewline.fmt(f)?; } } if !self.after_columns.is_empty() { - write!(f, "({}) ", display_comma_separated(&self.after_columns))?; + write!(f, "({})", display_comma_separated(&self.after_columns))?; + SpaceOrNewline.fmt(f)?; } if let Some(settings) = &self.settings { - write!(f, "SETTINGS {} ", display_comma_separated(settings))?; + write!(f, "SETTINGS {}", display_comma_separated(settings))?; + SpaceOrNewline.fmt(f)?; } if let Some(source) = &self.source { - write!(f, "{source}")?; + source.fmt(f)?; } else if !self.assignments.is_empty() { - write!(f, "SET ")?; - write!(f, "{}", display_comma_separated(&self.assignments))?; + write!(f, "SET")?; + indented_list(f, &self.assignments)?; } else if let Some(format_clause) = &self.format_clause { - write!(f, "{format_clause}")?; + format_clause.fmt(f)?; } else if self.columns.is_empty() { write!(f, "DEFAULT VALUES")?; } @@ -622,7 +626,9 @@ impl Display for Insert { } if let Some(returning) = &self.returning { - write!(f, " RETURNING {}", display_comma_separated(returning))?; + SpaceOrNewline.fmt(f)?; + f.write_str("RETURNING")?; + indented_list(f, returning)?; } Ok(()) } diff --git a/src/ast/mod.rs b/src/ast/mod.rs index a572e9d25..8e0fc4e22 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -4543,7 +4543,7 @@ impl fmt::Display for Statement { } Ok(()) } - Statement::Insert(insert) => write!(f, "{insert}"), + Statement::Insert(insert) => insert.fmt(f), Statement::Install { extension_name: name, } => write!(f, "INSTALL {name}"), diff --git a/src/ast/query.rs b/src/ast/query.rs index 33168695f..510d7d2b4 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -2883,14 +2883,26 @@ pub struct Values { impl fmt::Display for Values { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "VALUES ")?; - let prefix = if self.explicit_row { "ROW" } else { "" }; - let mut delim = ""; - for row in &self.rows { - write!(f, "{delim}")?; - delim = ", "; - write!(f, "{prefix}({})", display_comma_separated(row))?; + struct DisplayRow<'a> { + exprs: &'a [Expr], + explicit_row: bool, + } + impl<'a> fmt::Display for DisplayRow<'a> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let comma_separated = display_comma_separated(self.exprs); + if self.explicit_row { + write!(f, "ROW({})", comma_separated) + } else { + write!(f, "({})", comma_separated) + } + } } + let row_display_iter = self.rows.iter().map(|row| DisplayRow { + exprs: row, + explicit_row: self.explicit_row, + }); + f.write_str("VALUES")?; + indented_list(f, row_display_iter)?; Ok(()) } } diff --git a/src/display_utils.rs b/src/display_utils.rs index 28d8e415b..c151bd2e6 100644 --- a/src/display_utils.rs +++ b/src/display_utils.rs @@ -68,12 +68,17 @@ impl Display for SpaceOrNewline { /// A value that displays a comma-separated list of values. /// When pretty-printed (using {:#}), it displays each value on a new line. -pub(crate) struct DisplayCommaSeparated<'a, T: fmt::Display>(&'a [T]); +pub(crate) struct DisplayCommaSeparated(I) +where + I: IntoIterator + Clone; -impl fmt::Display for DisplayCommaSeparated<'_, T> { +impl fmt::Display for DisplayCommaSeparated +where + I: IntoIterator + Clone, +{ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let mut first = true; - for t in self.0 { + for t in self.0.clone() { if !first { f.write_char(',')?; SpaceOrNewline.fmt(f)?; @@ -86,9 +91,12 @@ impl fmt::Display for DisplayCommaSeparated<'_, T> { } /// Displays a whitespace, followed by a comma-separated list that is indented when pretty-printed. -pub(crate) fn indented_list(f: &mut fmt::Formatter, slice: &[T]) -> fmt::Result { +pub(crate) fn indented_list(f: &mut fmt::Formatter, iter: I) -> fmt::Result +where + I: IntoIterator + Clone, +{ SpaceOrNewline.fmt(f)?; - Indent(DisplayCommaSeparated(slice)).fmt(f) + Indent(DisplayCommaSeparated(iter)).fmt(f) } #[cfg(test)] diff --git a/tests/pretty_print.rs b/tests/pretty_print.rs index 9e68417d7..b4adbe352 100644 --- a/tests/pretty_print.rs +++ b/tests/pretty_print.rs @@ -171,7 +171,6 @@ string' AS str } #[test] -#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] fn test_pretty_print_insert_values() { assert_eq!( prettify("INSERT INTO my_table (a, b, c) VALUES (1, 2, 3), (4, 5, 6)"), @@ -186,10 +185,9 @@ VALUES } #[test] -#[ignore = "https://github.com/apache/datafusion-sqlparser-rs/issues/1850"] fn test_pretty_print_insert_select() { assert_eq!( - prettify("INSERT INTO my_table (a, b) SELECT x, y FROM source_table"), + prettify("INSERT INTO my_table (a, b) SELECT x, y FROM source_table RETURNING a AS id"), r#" INSERT INTO my_table (a, b) SELECT @@ -197,6 +195,8 @@ SELECT y FROM source_table +RETURNING + a AS id "# .trim() ); From 4bac8356d9b531b680bda0a725fd7c6f276d6d8b Mon Sep 17 00:00:00 2001 From: lovasoa Date: Wed, 14 May 2025 00:17:07 +0200 Subject: [PATCH 7/8] clippy --- src/ast/query.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ast/query.rs b/src/ast/query.rs index 510d7d2b4..2debd113b 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -2887,7 +2887,7 @@ impl fmt::Display for Values { exprs: &'a [Expr], explicit_row: bool, } - impl<'a> fmt::Display for DisplayRow<'a> { + impl fmt::Display for DisplayRow<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let comma_separated = display_comma_separated(self.exprs); if self.explicit_row { From 75ec6b95895a8cba6d6937a42c6e0a5eab968aa9 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Thu, 15 May 2025 08:07:25 +0200 Subject: [PATCH 8/8] remove generic indented_list implementation --- src/ast/query.rs | 27 ++++++++------------------- src/display_utils.rs | 18 +++++------------- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/src/ast/query.rs b/src/ast/query.rs index 2debd113b..ba0c079af 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -2883,26 +2883,15 @@ pub struct Values { impl fmt::Display for Values { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - struct DisplayRow<'a> { - exprs: &'a [Expr], - explicit_row: bool, - } - impl fmt::Display for DisplayRow<'_> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let comma_separated = display_comma_separated(self.exprs); - if self.explicit_row { - write!(f, "ROW({})", comma_separated) - } else { - write!(f, "({})", comma_separated) - } - } - } - let row_display_iter = self.rows.iter().map(|row| DisplayRow { - exprs: row, - explicit_row: self.explicit_row, - }); f.write_str("VALUES")?; - indented_list(f, row_display_iter)?; + let prefix = if self.explicit_row { "ROW" } else { "" }; + let mut delim = ""; + for row in &self.rows { + f.write_str(delim)?; + delim = ","; + SpaceOrNewline.fmt(f)?; + Indent(format_args!("{prefix}({})", display_comma_separated(row))).fmt(f)?; + } Ok(()) } } diff --git a/src/display_utils.rs b/src/display_utils.rs index c151bd2e6..849aea941 100644 --- a/src/display_utils.rs +++ b/src/display_utils.rs @@ -68,17 +68,12 @@ impl Display for SpaceOrNewline { /// A value that displays a comma-separated list of values. /// When pretty-printed (using {:#}), it displays each value on a new line. -pub(crate) struct DisplayCommaSeparated(I) -where - I: IntoIterator + Clone; +pub(crate) struct DisplayCommaSeparated<'a, T: fmt::Display>(&'a [T]); -impl fmt::Display for DisplayCommaSeparated -where - I: IntoIterator + Clone, -{ +impl fmt::Display for DisplayCommaSeparated<'_, T> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let mut first = true; - for t in self.0.clone() { + for t in self.0 { if !first { f.write_char(',')?; SpaceOrNewline.fmt(f)?; @@ -91,12 +86,9 @@ where } /// Displays a whitespace, followed by a comma-separated list that is indented when pretty-printed. -pub(crate) fn indented_list(f: &mut fmt::Formatter, iter: I) -> fmt::Result -where - I: IntoIterator + Clone, -{ +pub(crate) fn indented_list(f: &mut fmt::Formatter, items: &[T]) -> fmt::Result { SpaceOrNewline.fmt(f)?; - Indent(DisplayCommaSeparated(iter)).fmt(f) + Indent(DisplayCommaSeparated(items)).fmt(f) } #[cfg(test)]