Support SQL standard OVERLAY(... PLACING ... FROM ... FOR ...) syntax#101681
Conversation
…FOR length])` syntax Add parser-level syntactic sugar for the SQL standard `OVERLAY` function syntax. The `overlay` function already exists; this adds support for the keyword-based form using `PLACING`, `FROM`, and `FOR` as separators, matching PostgreSQL, Oracle, and DuckDB behavior. Both `overlay` and `overlayUTF8` support the new syntax. The existing comma-separated functional form continues to work unchanged. Closes ClickHouse#99604
alexey-milovidov
added
the
can be tested
clickhouse-gh
Bot
added
the
pr-improvement
|
The Stress test (arm_msan) failure is fixed by #101239, which should be merged first. After it is merged, please update the branch to include the fix. |
|
The failures of "Flaky check" in "functions_bad_arguments" will be fixed by #101994. |
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
|
|
||
| if (state == 1) | ||
| { | ||
| if (ParserToken(TokenType::Comma).ignore(pos, expected) |
There was a problem hiding this comment.
OverlayLayer currently accepts mixed separator styles (PLACING/FROM keywords interleaved with commas), because each separator check is comma || keyword independently.
This makes malformed SQL-standard forms execute as valid functional calls, e.g. OVERLAY('abcdef' PLACING 'XY', 3) gets parsed and executed as overlay('abcdef', 'XY', 3) instead of being rejected.
Please lock the syntax mode after the first separator (keyword mode vs comma mode) and only accept the corresponding separator family in subsequent states.
|
|
||
| /// Allow closing bracket in any state so that calls with too few arguments | ||
| /// (e.g. overlay('a', 'b')) are rejected by the function, not the parser. | ||
| if (!finished && ParserToken(TokenType::ClosingRoundBracket).ignore(pos, expected)) |
There was a problem hiding this comment.
ClosingRoundBracket is accepted unconditionally in any state. In the SQL-standard form this allows missing mandatory FROM to slip through parsing.
Concrete case: OVERLAY('abcdef' PLACING 'XY') currently parses as overlay('abcdef', 'XY') and fails only with NUMBER_OF_ARGUMENTS_DOESNT_MATCH, while the query is syntactically invalid and should be rejected at parse time.
Please allow early ) only for the legacy comma form where it is intentional; for keyword mode, require FROM before accepting a closing bracket.
…yntax Address review feedback: - Track keyword_mode vs comma_mode after the first separator - In keyword mode, only accept PLACING/FROM/FOR keywords - In comma mode, only accept commas - In keyword mode, require FROM before allowing closing bracket - Add negative test cases for mixed syntax rejection
…tations Syntax errors from the parser are caught client-side by clickhouse-client before the query reaches the server, so `clientError` is the correct annotation for these test cases.
LLVM Coverage Report
Changed lines: 92.00% (92/100) · Uncovered code |
robot-ch-test-poll4
added
the
pr-synced-to-cloud
|
@Desel72 Please add documentation for |
|
@Desel72 Please add documentation for OVERLAY here. |
4 participants
…FOR length])` syntax
Add parser-level syntactic sugar for the SQL standard
OVERLAYfunction syntax. Theoverlayfunction already exists; this adds support for the keyword-based form usingPLACING,FROM, andFORas separators, matching PostgreSQL, Oracle, and DuckDB behavior.Both
overlayandoverlayUTF8support the new syntax. The existing comma-separated functional form continues to work unchanged.Closes #99604
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Add parser-level syntactic sugar for the SQL standard
OVERLAYfunction syntax. Theoverlayfunction already exists; this adds support for the keyword-based form usingPLACING,FROM, andFORas separators.Documentation entry for user-facing changes
Version info
26.4.1.769