Conversation
…ansformers
Adds a dedicated select-item branch for a COLUMNS('regexp') matcher followed
by one or more transformers, fixing JSQLParser#2631 (APPLY with a lambda such as
x -> round(x, 2) failed to parse; without a lambda, APPLY(...) was silently
mis-parsed as the select item alias).
- new ColumnsExpression / ColumnsTransformer AST nodes with an ordered
transformer list, since ClickHouse parses transformers in a loop (they
may repeat and combine in any order)
- semantic lookahead isColumnsExpressionAhead() keeps a plain
COLUMNS('regexp') on the regular Function path
- expression visitor, adapter, deparser, validator and TablesNamesFinder
traverse the new node
Signed-off-by: 付典 <fudianchn@gmail.com>
Contributor
|
But is it turing complete? |
Satisfies the PMD exhaustive-switch rule (Codacy '1 new issue') and fails loudly if a future ColumnsTransformerType constant is added without updating appendTo; unreachable for the current APPLY/EXCEPT/REPLACE constants, so behavior is unchanged. Same pattern as JsonFunction's Unhandled JsonOnResponseBehavior guard. Signed-off-by: 付典 <fudianchn@gmail.com>
Contributor
Author
|
Hahaha, obviously I haven't made the phone call yet. |
Reuse alias grammar at each transformer boundary, preserve JJTree parents and lookahead visibility, and route all child expressions through visitor hooks. Add consumer and grammar regression coverage. Signed-off-by: 付典 <fudianchn@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI disclosure: this change was prepared with AI coding agents, reviewed and revised line by line by me.
What
Parse ClickHouse
COLUMNS('regexp')select items with ordered, repeatableAPPLY(...),EXCEPT (...), andREPLACE (...)transformers, includingAPPLY(x -> round(x, 2)). See the ClickHouse SELECT documentation.Why
The query from #2631 fails on the lambda arrow. The simpler
COLUMNS('m') APPLY(sum)previously parsed as a function with the aliasAPPLY(sum), hiding the missing transformer support.How
ColumnsExpressionholds the matcher and its orderedColumnsTransformerlist.toString. The new ExpressionVisitor overload has a default implementation.Root cause
The grammar lacked column-transformer syntax and fell through to aliases. Self-review of the initial patch also found incomplete child traversal and heuristic alias detection that rejected valid array, IN, and CASE lambda bodies. The revised implementation addresses both parser and consumer behavior.
Testing
./gradlew checkpassed (6,824 tests, zero failures/errors, 25 skipped);mvn clean verifypassed (6,806 tests, zero failures/errors, 25 skipped). Grammar ambiguity, formatting, Checkstyle, PMD, SpotBugs, and applicable coverage checks passed.parseSQLStatements, unchangedperformance.sql,version=latest, JDK 17.0.20 on an i9-13900KS host (32 logical CPUs): 10 forks × 10 one-second measurements, after 3 one-second warmups, per run. Interleaved base/branch ×2: base 21.770±0.139 / 21.923±0.128 versus branch 21.864±0.132 / 21.728±0.102 ms/op (JMH 99.9% CI). The pooled difference is -0.23%, within noise; no measurable regression in this benchmark. Both parsers returned all 54 statements before measurement. The comparison uses PR base537f3a6dand identical benchmark bytecode, not the subsequently modified master harness.Behavior notes
APPLY(sum)form becomes a transformer rather than an alias.REPLACE(a)/REPLACE(a, b)alias lists, and EXCEPT set operations retain their existing interpretations. ExplicitAS APPLY(sum)remains an alias.Verification of the original issue
On PR base
537f3a6d,SELECT COLUMNS('^metric_') APPLY(x -> round(x, 2)) FROM metricsfails to parse. With this change it produces a ColumnsExpression with a LambdaExpression and round-trips. The issue does not specify a JSqlParser version; no claim is made about running that unspecified version or a ClickHouse server.Fixes #2631