You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[BUG] 5.4-SNAPSHOT : ClickHouse : asterisk column modifiers fail or mis-parse (APPLY as alias, docs chain, STRICT, EXCEPT regex) #2636
AI disclosure: this issue was prepared with AI coding agents, revised and verified by me.
Failing SQL Feature:
ClickHouse column modifiers on * / t.*: the documented APPLY modifier is silently mis-parsed as a select-item alias, the documented modifier chain from the SELECT docs fails with a ParseException, and the STRICT variants plus the regex-string form of EXCEPT (implemented in the ClickHouse grammar, covered by its stateless tests) are unsupported.
SQL Example:
-- 1) documented in the ClickHouse SELECT docs, mis-parsed on master:-- parses as AllColumns with alias "APPLY(sum)"SELECT* APPLY(sum) FROM t;
-- 2) the modifier chain exactly as shown in the ClickHouse docs, fails:SELECT* REPLACE(i +1AS i) EXCEPT (j) APPLY(sum) FROM columns_transformers;
-- 3) STRICT variants (undocumented, but in the ClickHouse grammar and-- its stateless tests) and the regex form of EXCEPT, all fail:SELECT* EXCEPT STRICT (a, b) FROM t;
SELECT* REPLACE STRICT (1AS a, 2AS b) FROM t;
SELECT* EXCEPT '^tmp_'FROM t;
Baseline that already parses: SELECT * EXCEPT (a, b) FROM t, SELECT * REPLACE (a AS b) FROM t, SELECT * EXCLUDE (a) FROM t.
Software Information:
JSqlParser version: current master 5.4-SNAPSHOT, commit 02b9d94 (tested 2026-09-14)
Database: ClickHouse
Additional context
ClickHouse grammar accepts STRICT after both EXCEPT and REPLACE, and a string-literal regex pattern for EXCEPT (ExpressionElementParsers.cpp: is_strict, ParserStringLiteral -> ASTColumnsExceptTransformer::setPattern, formatted as EXCEPT STRICT in ASTColumnsTransformers.cpp); covered by the stateless tests 01470_columns_transformers.sql and 02343_analyzer_column_transformers_strict.sql.
Proposed scope — only the first two items, to keep the patch reviewable:
SELECT * APPLY(sum): parse as AllColumns + ColumnsTransformer instead of falling into the alias path (APPLY is in NonReservedWord(), so it currently becomes the alias "APPLY(sum)").
The modifier chain from the ClickHouse docs (* EXCEPT (a) REPLACE (b AS c)).
I'd leave STRICT and the regex form of EXCEPT for a follow-up — they look like separate grammar work, and I'd rather not bundle four behaviours into one diff.
Approach: extend the AllColumns(boolean) production in JSqlParserCC.jjt (~L7946) to accept a repeated ColumnsTransformer(), reuse the existing ColumnsExpression / ColumnsTransformer classes, then update AllColumns, the ExpressionVisitor / DeParser pair, and add round-trip tests via assertSqlCanBeParsedAndDeparsed().
Heads-up: #2652 also touches AllColumns.java — happy to rebase after it lands, or hold off if you'd prefer.
AI disclosure: I use an AI coding assistant; I review and test everything I submit and take responsibility for it.
Follow-up on the approach — one thing I'd like to get right before writing code.
AllColumns currently models these modifiers itself (exceptColumns / replaceExpressions / exceptKeyword), while ColumnsTransformer (from #2635) models the same keywords for the COLUMNS(...) path. If I give AllColumns a List<ColumnsTransformer>, the same syntax ends up with two representations.
Which would you prefer?
Unify on ColumnsTransformer — AllColumns holds a List<ColumnsTransformer> and the three legacy fields go away. Consistent with ColumnsExpression, and chains like * EXCEPT (a) REPLACE (b AS c) fall out of the repeated-transformer grammar for free. Cost: it's a breaking API change, and AllTableColumns extends AllColumns so its constructors change too. Internal usage looks small — AllTableColumns reads getExceptKeyword(), plus the deparser and one test.
Keep the legacy fields, add transformers only for the new cases — non-breaking, but two representations for EXCEPT / REPLACE.
I lean (1) since 5.4 is still a snapshot, but I don't want to churn public API without a nod.
One more question: should * APPLY(...) and table.* APPLY(...) both be supported? The issue only shows the plain * form.
Activity
kyy-logs commentedon Sep 19, 2026
I'd like to take this one.
Proposed scope — only the first two items, to keep the patch reviewable:
SELECT * APPLY(sum): parse asAllColumns+ColumnsTransformerinstead of falling into the alias path (APPLYis inNonReservedWord(), so it currently becomes the alias"APPLY(sum)").* EXCEPT (a) REPLACE (b AS c)).I'd leave
STRICTand the regex form ofEXCEPTfor a follow-up — they look like separate grammar work, and I'd rather not bundle four behaviours into one diff.Approach: extend the
AllColumns(boolean)production inJSqlParserCC.jjt(~L7946) to accept a repeatedColumnsTransformer(), reuse the existingColumnsExpression/ColumnsTransformerclasses, then updateAllColumns, theExpressionVisitor/DeParserpair, and add round-trip tests viaassertSqlCanBeParsedAndDeparsed().Heads-up: #2652 also touches
AllColumns.java— happy to rebase after it lands, or hold off if you'd prefer.AI disclosure: I use an AI coding assistant; I review and test everything I submit and take responsibility for it.
kyy-logs commentedon Sep 19, 2026
Follow-up on the approach — one thing I'd like to get right before writing code.
AllColumnscurrently models these modifiers itself (exceptColumns/replaceExpressions/exceptKeyword), whileColumnsTransformer(from #2635) models the same keywords for theCOLUMNS(...)path. If I giveAllColumnsaList<ColumnsTransformer>, the same syntax ends up with two representations.Which would you prefer?
ColumnsTransformer—AllColumnsholds aList<ColumnsTransformer>and the three legacy fields go away. Consistent withColumnsExpression, and chains like* EXCEPT (a) REPLACE (b AS c)fall out of the repeated-transformer grammar for free. Cost: it's a breaking API change, andAllTableColumns extends AllColumnsso its constructors change too. Internal usage looks small —AllTableColumnsreadsgetExceptKeyword(), plus the deparser and one test.EXCEPT/REPLACE.I lean (1) since 5.4 is still a snapshot, but I don't want to churn public API without a nod.
One more question: should
* APPLY(...)andtable.* APPLY(...)both be supported? The issue only shows the plain*form.