Conversation
Signed-off-by: Duc Canh Le <duccanh.le@ahrefs.com> Co-authored-by: Dmitry Novik <n0vik@clickhouse.com>
|
Workflow [PR], commit [8457ed6] Summary: ❌
AI ReviewSummaryThis PR adds experimental Findings
ClickHouse Rules
Final Verdict
|
| Prewhere filter | ||
| Prewhere filter column: greaterOrEquals(a, 0) | ||
| Filter column: and(indexHint(less(i, 100)), less(multiply(2, b), 100)) (removed) | ||
| Filter column: and(less(multiply(2, b), 100), indexHint(less(i, 100))) (removed) |
There was a problem hiding this comment.
The order of conditions somehow depends on the hash value during conversion to CNF. I think it's okay because it reverts the change from #82617.
There was a problem hiding this comment.
I see the order changes all the time, probably not for the last time then.
4a33dfd to
083c9c7
Compare
…icas external tables
|
Fix for |
|
Inconsistent AST formatting - fixed in #100174 |
| /// If table is not removed from Context, it would cause an exception in distributed queries, | ||
| /// because the temporary table would already exist in the Context | ||
| /// and there would be an attempt to read it to send the temporary table to remote servers. | ||
| getContext()->getQueryContext()->removeExternalTable(table_node->getTemporaryTableName()); |
There was a problem hiding this comment.
inlineMaterializedCTEIfNeeded clears reused_materialized_cte when context->hasQueryContext() is false, but InlineMaterializedCTEsVisitor::enterImpl still unconditionally calls getContext()->getQueryContext()->removeExternalTable(...) for non-reused CTEs.
That throws THERE_IS_NO_QUERY in analyzer-only contexts and can fail queries that use MATERIALIZED CTEs without a query context.
Please guard this call with hasQueryContext() (or skip external-table removal entirely when there is no query context).
LLVM Coverage Report
PR changed lines: PR changed-lines coverage: 94.19% (924/981, 6 noise lines excluded) |
Cherry pick #94849 to 26.3: Support Materialized CTE
Backport #94849 to 26.3: Support Materialized CTE
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Support Materialized CTE. Allow evaluating CTEs only once during query execution and store their results in temporary tables. Closes #53449.
Documentation entry for user-facing changes
This PR used one commit and some test queries written by @canhld94 in #61086. Closes #61086.
Note
High Risk
Touches core analyzer/planner/optimizer and execution pipeline to introduce new temp-table materialization paths and a new setting, which can affect query correctness/performance and distributed/parallel replicas behavior.
Overview
Adds experimental materialized CTEs via
WITH name AS MATERIALIZED (subquery), guarded by new settingenable_materialized_cte.The analyzer/planner now tracks
is_materializedonQueryNode/UnionNode, parses/formats the new syntax, resolves materialized CTE identifiers by converting them toTableNodes backed by temporaryStorageMemory, forbids correlated materialized CTEs, and errors when combined withWITH RECURSIVE.Query planning/execution gains CTE materialization steps (
MaterializingCTETransform,MaterializingCTEStep, delayed planning + optimization pass) to run CTE subplans once before the main query, coordinate reuse across sets/PK analysis/distributed queries, and inline single-use materialized CTEs back to avoid external-table conflicts; adds tests and docs for behavior and edge cases.Written by Cursor Bugbot for commit 1753b65. This will update automatically on new commits. Configure here.