From 3c96aa8e5137b7ae2e7226ea3ff4c083f10748fb Mon Sep 17 00:00:00 2001 From: erezrokah Date: Mon, 11 May 2026 08:56:40 +0100 Subject: [PATCH] fix: Allow trailing comma in SELECT list before clause keyword ClickHouse accepts `SELECT a, FROM t` (e.g. when a column is commented out with `--`). Break the SELECT-item loop after consuming a comma if the next token is a clause-starting keyword (FROM, WHERE, GROUP, etc.). Fixes incorrect AST for existing CREATE MATERIALIZED VIEW fixtures where FROM was previously consumed as a bare select expression. --- parser/parser_column.go | 3 + .../create_materialized_view_with_definer.sql | 5 +- .../create_materialized_view_with_refresh.sql | 5 +- .../create_materialized_view_with_definer.sql | 2 +- .../create_materialized_view_with_refresh.sql | 2 +- ...rialized_view_with_definer.sql.golden.json | 39 +++++---- ...rialized_view_with_refresh.sql.golden.json | 39 +++++---- .../select_trailing_comma_before_from.sql | 10 +++ .../select_trailing_comma_before_from.sql | 7 ++ ...trailing_comma_before_from.sql.golden.json | 82 +++++++++++++++++++ .../select_trailing_comma_before_from.sql | 2 + 11 files changed, 158 insertions(+), 38 deletions(-) create mode 100644 parser/testdata/query/format/beautify/select_trailing_comma_before_from.sql create mode 100644 parser/testdata/query/format/select_trailing_comma_before_from.sql create mode 100644 parser/testdata/query/output/select_trailing_comma_before_from.sql.golden.json create mode 100644 parser/testdata/query/select_trailing_comma_before_from.sql diff --git a/parser/parser_column.go b/parser/parser_column.go index 346a6962..ef96cccf 100644 --- a/parser/parser_column.go +++ b/parser/parser_column.go @@ -587,6 +587,9 @@ func (p *Parser) parseSelectItems() ([]*SelectItem, error) { if p.tryConsumeTokenKind(TokenKindComma) == nil { break } + if p.isSelectItemTerminatorKeyword() { + break + } } return selectItems, nil } diff --git a/parser/testdata/ddl/format/beautify/create_materialized_view_with_definer.sql b/parser/testdata/ddl/format/beautify/create_materialized_view_with_definer.sql index 905e2066..15fe7713 100644 --- a/parser/testdata/ddl/format/beautify/create_materialized_view_with_definer.sql +++ b/parser/testdata/ddl/format/beautify/create_materialized_view_with_definer.sql @@ -32,8 +32,9 @@ AS (SELECT timestamp, field_1, - field_2, - FROM AS event_table + field_2 + FROM + event_table WHERE toStartOfHour(timestamp) = toStartOfHour(now() - toIntervalHour(1))) COMMENT 'Test comment'; diff --git a/parser/testdata/ddl/format/beautify/create_materialized_view_with_refresh.sql b/parser/testdata/ddl/format/beautify/create_materialized_view_with_refresh.sql index fac39199..2d4f42b8 100644 --- a/parser/testdata/ddl/format/beautify/create_materialized_view_with_refresh.sql +++ b/parser/testdata/ddl/format/beautify/create_materialized_view_with_refresh.sql @@ -31,5 +31,6 @@ AS SELECT `field_1`, `field_2`, - `field_3`, - FROM AS table_v5; + `field_3` + FROM + table_v5; diff --git a/parser/testdata/ddl/format/create_materialized_view_with_definer.sql b/parser/testdata/ddl/format/create_materialized_view_with_definer.sql index 00ad9d0a..298722a6 100644 --- a/parser/testdata/ddl/format/create_materialized_view_with_definer.sql +++ b/parser/testdata/ddl/format/create_materialized_view_with_definer.sql @@ -17,4 +17,4 @@ COMMENT 'Test comment' -- Format SQL: -CREATE MATERIALIZED VIEW fresh_mv REFRESH EVERY 1 HOUR OFFSET 10 MINUTE APPEND TO events_export (`timestamp` DateTime64(9), `field_1` String, `field_2` String) DEFINER = default SQL SECURITY DEFINER AS (SELECT timestamp, field_1, field_2, FROM AS event_table WHERE toStartOfHour(timestamp) = toStartOfHour(now() - toIntervalHour(1))) COMMENT 'Test comment'; +CREATE MATERIALIZED VIEW fresh_mv REFRESH EVERY 1 HOUR OFFSET 10 MINUTE APPEND TO events_export (`timestamp` DateTime64(9), `field_1` String, `field_2` String) DEFINER = default SQL SECURITY DEFINER AS (SELECT timestamp, field_1, field_2 FROM event_table WHERE toStartOfHour(timestamp) = toStartOfHour(now() - toIntervalHour(1))) COMMENT 'Test comment'; diff --git a/parser/testdata/ddl/format/create_materialized_view_with_refresh.sql b/parser/testdata/ddl/format/create_materialized_view_with_refresh.sql index 7dbb3878..463fa40c 100644 --- a/parser/testdata/ddl/format/create_materialized_view_with_refresh.sql +++ b/parser/testdata/ddl/format/create_materialized_view_with_refresh.sql @@ -16,4 +16,4 @@ AS SELECT FROM table_v5 -- Format SQL: -CREATE MATERIALIZED VIEW fresh_mv REFRESH EVERY 1 HOUR OFFSET 10 MINUTE RANDOMIZE FOR 1 SECOND DEPENDS ON table_v5 SETTINGS randomize_for=1, randomize_offset=10, randomize_period=1 APPEND TO target_table_name EMPTY AS SELECT `field_1`, `field_2`, `field_3`, FROM AS table_v5; +CREATE MATERIALIZED VIEW fresh_mv REFRESH EVERY 1 HOUR OFFSET 10 MINUTE RANDOMIZE FOR 1 SECOND DEPENDS ON table_v5 SETTINGS randomize_for=1, randomize_offset=10, randomize_period=1 APPEND TO target_table_name EMPTY AS SELECT `field_1`, `field_2`, `field_3` FROM table_v5; diff --git a/parser/testdata/ddl/output/create_materialized_view_with_definer.sql.golden.json b/parser/testdata/ddl/output/create_materialized_view_with_definer.sql.golden.json index ec66da9f..3b033c83 100644 --- a/parser/testdata/ddl/output/create_materialized_view_with_definer.sql.golden.json +++ b/parser/testdata/ddl/output/create_materialized_view_with_definer.sql.golden.json @@ -212,24 +212,31 @@ }, "Modifiers": [], "Alias": null - }, - { - "Expr": { - "Name": "FROM", - "QuoteType": 1, - "NamePos": 266, - "NameEnd": 270 - }, - "Modifiers": [], - "Alias": { - "Name": "event_table", - "QuoteType": 1, - "NamePos": 271, - "NameEnd": 282 - } } ], - "From": null, + "From": { + "FromPos": 266, + "Expr": { + "Table": { + "TablePos": 271, + "TableEnd": 282, + "Alias": null, + "Expr": { + "Database": null, + "Table": { + "Name": "event_table", + "QuoteType": 1, + "NamePos": 271, + "NameEnd": 282 + } + }, + "HasFinal": false + }, + "StatementEnd": 282, + "SampleRatio": null, + "HasFinal": false + } + }, "Window": null, "Prewhere": null, "Where": { diff --git a/parser/testdata/ddl/output/create_materialized_view_with_refresh.sql.golden.json b/parser/testdata/ddl/output/create_materialized_view_with_refresh.sql.golden.json index f9c877ed..bf7e9461 100644 --- a/parser/testdata/ddl/output/create_materialized_view_with_refresh.sql.golden.json +++ b/parser/testdata/ddl/output/create_materialized_view_with_refresh.sql.golden.json @@ -179,24 +179,31 @@ }, "Modifiers": [], "Alias": null - }, - { - "Expr": { - "Name": "FROM", - "QuoteType": 1, - "NamePos": 289, - "NameEnd": 293 - }, - "Modifiers": [], - "Alias": { - "Name": "table_v5", - "QuoteType": 1, - "NamePos": 294, - "NameEnd": 302 - } } ], - "From": null, + "From": { + "FromPos": 289, + "Expr": { + "Table": { + "TablePos": 294, + "TableEnd": 302, + "Alias": null, + "Expr": { + "Database": null, + "Table": { + "Name": "table_v5", + "QuoteType": 1, + "NamePos": 294, + "NameEnd": 302 + } + }, + "HasFinal": false + }, + "StatementEnd": 302, + "SampleRatio": null, + "HasFinal": false + } + }, "Window": null, "Prewhere": null, "Where": null, diff --git a/parser/testdata/query/format/beautify/select_trailing_comma_before_from.sql b/parser/testdata/query/format/beautify/select_trailing_comma_before_from.sql new file mode 100644 index 00000000..519a4192 --- /dev/null +++ b/parser/testdata/query/format/beautify/select_trailing_comma_before_from.sql @@ -0,0 +1,10 @@ +-- Origin SQL: +SELECT count(x), --name +FROM cloud_assets + + +-- Beautify SQL: +SELECT + count(x) +FROM + cloud_assets; diff --git a/parser/testdata/query/format/select_trailing_comma_before_from.sql b/parser/testdata/query/format/select_trailing_comma_before_from.sql new file mode 100644 index 00000000..2146e697 --- /dev/null +++ b/parser/testdata/query/format/select_trailing_comma_before_from.sql @@ -0,0 +1,7 @@ +-- Origin SQL: +SELECT count(x), --name +FROM cloud_assets + + +-- Format SQL: +SELECT count(x) FROM cloud_assets; diff --git a/parser/testdata/query/output/select_trailing_comma_before_from.sql.golden.json b/parser/testdata/query/output/select_trailing_comma_before_from.sql.golden.json new file mode 100644 index 00000000..e540a078 --- /dev/null +++ b/parser/testdata/query/output/select_trailing_comma_before_from.sql.golden.json @@ -0,0 +1,82 @@ +[ + { + "SelectPos": 0, + "StatementEnd": 41, + "With": null, + "Top": null, + "HasDistinct": false, + "DistinctOn": null, + "SelectItems": [ + { + "Expr": { + "Name": { + "Name": "count", + "QuoteType": 1, + "NamePos": 7, + "NameEnd": 12 + }, + "Params": { + "LeftParenPos": 12, + "RightParenPos": 14, + "Items": { + "ListPos": 13, + "ListEnd": 14, + "HasDistinct": false, + "Items": [ + { + "Expr": { + "Name": "x", + "QuoteType": 1, + "NamePos": 13, + "NameEnd": 14 + }, + "Alias": null + } + ] + }, + "ColumnArgList": null + } + }, + "Modifiers": [], + "Alias": null + } + ], + "From": { + "FromPos": 24, + "Expr": { + "Table": { + "TablePos": 29, + "TableEnd": 41, + "Alias": null, + "Expr": { + "Database": null, + "Table": { + "Name": "cloud_assets", + "QuoteType": 1, + "NamePos": 29, + "NameEnd": 41 + } + }, + "HasFinal": false + }, + "StatementEnd": 41, + "SampleRatio": null, + "HasFinal": false + } + }, + "Window": null, + "Prewhere": null, + "Where": null, + "GroupBy": null, + "WithTotal": false, + "Having": null, + "OrderBy": null, + "LimitBy": null, + "Limit": null, + "Settings": null, + "Format": null, + "UnionAll": null, + "UnionDistinct": null, + "Except": null + } +] \ No newline at end of file diff --git a/parser/testdata/query/select_trailing_comma_before_from.sql b/parser/testdata/query/select_trailing_comma_before_from.sql new file mode 100644 index 00000000..44ed1d52 --- /dev/null +++ b/parser/testdata/query/select_trailing_comma_before_from.sql @@ -0,0 +1,2 @@ +SELECT count(x), --name +FROM cloud_assets