Remove the kql table function#105101
Conversation
The `kql(...)` table function was a low-quality bolted-on feature that hijacked function parsing in `ExpressionListParsers.cpp` and rewrote itself into a `view(...)` call. It is removed entirely. The KQL dialect (`SET dialect = 'kusto'`) remains supported. The internal `ParserKQLTableFunction` class — which is still needed by the KQL dialect to parse parenthesized KQL subqueries (e.g. inside the `in` operator) — is renamed to `ParserKQLParenExpression` to reflect what it actually does now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
clickhouse-gh
Bot
added
the
pr-backward-incompatible
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
| return std::make_unique<ViewLayer>(true); | ||
| if (function_name_lowercase == "kql") | ||
| return std::make_unique<KustoLayer>(); | ||
| } |
There was a problem hiding this comment.
This removes the kql table-function hook, but both direct kql(...) stateless tests are deleted in this PR and there is no replacement that asserts the new contract.
Please add a focused negative stateless test (for example, SELECT * FROM kql(...)) that verifies we now return UNKNOWN_TABLE_FUNCTION. For a backward-incompatible removal, that regression coverage is important so this behavior does not silently come back.
There was a problem hiding this comment.
We don't need that.
There was a problem hiding this comment.
This is still missing in the current diff: both former kql(...) stateless tests were deleted and there is no replacement check that asserts the new contract (kql(...) must fail with UNKNOWN_TABLE_FUNCTION).
For a backward-incompatible removal, please add a focused negative stateless test (for example SELECT * FROM kql('x')) with serverError UNKNOWN_TABLE_FUNCTION so this behavior cannot be reintroduced silently.
|
@groeneai, the following CI failures on this PR are unrelated to the change (removal of the
|
|
Thanks @alexey-milovidov — all three failures confirmed unrelated to this PR (KQL parser removal). Status of each: 1. 2. 3. This PR is safe to merge as soon as the rest of CI completes. |
|
@groeneai, please focus on "FunctionsStress.stress — arrayResize 79s timeout on max_execution_time=15s" - find a way to guard/limit these heavy invocations in the implementation and send a PR. |
LLVM Coverage Report
Changed lines: 100.00% (20/20) | lost baseline coverage: 2 line(s) · Uncovered code |
robot-clickhouse-ci-1
added
the
pr-synced-to-cloud
3 participants
The
kql(...)table function was a low-quality bolted-on feature that hijacked function parsing inExpressionListParsers.cppand rewrote itself into aview(...)call. It is removed entirely. The KQL dialect (SET dialect = 'kusto') remains supported.The internal
ParserKQLTableFunctionclass — which is still needed by the KQL dialect to parse parenthesized KQL subqueries (e.g. inside theinoperator) — is renamed toParserKQLParenExpressionto reflect what it actually does now.Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Removed the
kqltable function. UseSET dialect = 'kusto'to run queries in the KQL dialect.Documentation entry for user-facing changes
Version info
26.5.1.725