Apply AST size limits to the query generated by table function eval

[groeneai]

Changelog category (leave one):

• Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Apply the AST size limits max_ast_depth and max_ast_elements to the query generated by the eval table function, same as for a query executed directly.

Description

Follow-up to #110132 (per @ alexey-milovidov's review request on src/TableFunctions/TableFunctionEval.cpp).

eval parses and stores the generated inner SELECT but, unlike executeQueryImpl, never applied the AST size limits. That made max_ast_depth / max_ast_elements ineffective for the inner query: a tiny outer SELECT * FROM eval('...') could smuggle a huge or very deep AST into the analyzer, even though executing the same inner query directly would be rejected.

This applies checkDepth/checkSize to the generated query, reading the limits from a context that already has the generated query's own SETTINGS applied. The settings are resolved before the normalization visitors rewrite the query tree (which can move or drop the SETTINGS clause), so an inner ... SETTINGS max_ast_elements = N controls its own limits both to tighten and to relax them, the same way it would for a standalone query.

Regression tests added to 04512_eval_table_function.

[groeneai]

Pre-PR validation gate (click to expand)

#	Question	Answer
a	Deterministic repro?	Yes. SELECT count() FROM eval('SELECT 1+2+...+10') SETTINGS max_ast_elements = 30 returns a result on master (limit ignored) and throws TOO_BIG_AST with the fix. Same for a deep inner query with max_ast_depth.
b	Root cause explained?	eval parses/stores the generated inner SELECT but, unlike executeQueryImpl, never applied the AST size limits, so max_ast_depth/max_ast_elements were skipped for the inner query.
c	Fix matches root cause?	Yes. Applies checkDepth/checkSize to the generated query, mirroring executeQueryImpl's checkASTSizeLimits.
d	Test intent preserved / new tests added?	New regression cases added to 04512_eval_table_function (tighten, relax, and inner-SETTINGS override). Existing assertions unchanged.
e	Demonstrated in both directions?	Yes. Fails on pristine build (Build ID d69aa46, no rejection); passes on fixed build (f2dd557, rejects). clickhouse-test 04512_eval_table_function = OK.
f	Fix is general, not a narrow patch?	The limits are applied to the whole generated SELECT/UNION tree; covers depth and element count. eval is the only table function that parses a runtime query string, so no sibling paths.
g	Generalizes across inputs?	Both limits covered; inner SETTINGS (tighten and relax) and UNION-form queries verified.
h	Backward compatible?	Yes. No setting default or format change; only enforces existing limits on a path that previously skipped them.
i	Invariants/contracts preserved?	Settings resolved into a private Context::createCopy before the normalization visitors rewrite the tree; outer context is untouched.

Session id: cron:clickhouse-worker-slot-0:20260713-042100

[groeneai]

cc @alexey-milovidov — follow-up to #110132 as requested: applies the AST size limits to the query generated by eval.

[clickhouse-gh]

Workflow [PR], commit [2ad4949]

Summary: ❌

job_name	test_name	status	info	comment
Finish Workflow		FAIL
	python3 ./ci/jobs/scripts/workflow_hooks/new_tests_check.py	FAIL
Config Workflow		ERROR
Dockers Build (amd)		DROPPED
Dockers Build (arm)		DROPPED
Dockers Build (multiplatform manifest)		DROPPED
Style check		DROPPED
Code Review		DROPPED
Docs check		DROPPED
Docs check (Mintlify)		DROPPED
Fast test		DROPPED

[groeneai]

Fixed in f502546. The set-operation normalization visitors now read intersect_default_mode / except_default_mode / union_default_mode from limits_context->getSettingsRef() (the generated query's own resolved SETTINGS) instead of the outer context, same as the AST size limits already do. So an inner ... SETTINGS union_default_mode = 'DISTINCT' normalizes exactly as it would when the query is executed directly.

Verified both directions with your repro on a debug build:

• Without the fix: SELECT count() FROM eval('SELECT 1 AS n UNION SELECT 1 AS n SETTINGS union_default_mode = ''DISTINCT''') throws EXPECTED_ALL_OR_DISTINCT.
• With the fix: returns 1. The ambiguous ... UNION ... with no inner SETTINGS still throws EXPECTED_ALL_OR_DISTINCT (outer default preserved).

Added a regression in 04512_eval_table_function covering the inner union_default_mode = 'DISTINCT' case plus a paired assertion that the ambiguous UNION without inner settings still errors.

Pre-PR validation gate (click to expand)

#	Question	Answer
a	Deterministic repro?	Yes — the exact query in the review throws EXPECTED_ALL_OR_DISTINCT on demand before the fix.
b	Root cause explained?	The visitors at TableFunctionEval.cpp:166-172 drove SelectIntersectExceptQueryVisitor / NormalizeSelectWithUnionQueryVisitor from the OUTER settings, so NormalizeSelectWithUnionQueryVisitor saw an empty outer union_default_mode and rejected an ambiguous UNION even though the inner query set it to DISTINCT.
c	Fix matches root cause?	Yes — the visitors now read the three set-operation modes from limits_context (the inner query's resolved SETTINGS), matching how the AST size limits are already resolved.
d	Test intent preserved / new tests added?	Yes — added a regression for the inner-DISTINCT case and kept the outer-default ambiguous UNION error assertion, so both behaviors are pinned.
e	Both directions demonstrated?	Yes — fails (EXPECTED_ALL_OR_DISTINCT) without the fix, passes (1) with it; test OK against the fixed binary (Build ID a2f45ef).
f	Fix is general across code paths?	Yes — all three set-operation modes (intersect/except/union) are threaded from the inner context together; there is a single normalization site.
g	Fix generalizes across inputs?	Covered the parallel modes: intersect_default_mode and except_default_mode are switched to the inner context in the same change as union_default_mode, so an inner SETTINGS clause controls all three the same way a direct query would.
h	Backward compatible?	Yes — no setting default, format, or validation change; only which context the inner query's own SETTINGS are read from during normalization.
i	Invariants and contracts preserved?	Yes — limits_context is a private copy already used for the AST-size limits; the modes+selects invariant of the union AST is unaffected (normalization runs once, before the size check, same order as before).

Session id: cron:clickhouse-worker-slot-2:20260713-083200

[groeneai]

Fixed in efef3e9. eval now runs ApplyWithGlobalVisitor (gated by the inner query's own enable_global_with_statement) before checkDepth/checkSize, mirroring executeQueryImpl ordering, so the size limits see the post-WITH-expansion AST. Regression added to 04512_eval_table_function.

Pre-PR validation gate (click to expand)

#	Question	Answer
a	Deterministic repro?	Yes. eval('WITH 1+2+...+15 AS big SELECT big UNION ALL SELECT big x8 SETTINGS max_ast_elements = 100') passed in eval (pre-fix) while the same text executed directly throws TOO_BIG_AST.
b	Root cause explained?	eval checked max_ast_depth/max_ast_elements on the PRE-WITH-expansion AST. executeQueryImpl runs ApplyWithGlobalVisitor (under enable_global_with_statement) first, so a global CTE inlined into every branch grows past the limit only after expansion; eval skipped that step and let the over-limit query through.
c	Fix matches root cause?	Yes. Added ApplyWithGlobalVisitor::visit(query) gated by the inner query's enable_global_with_statement, ordered before the size checks, exactly as executeQueryImpl does.
d	Test intent preserved / new tests added?	New regression added; no assertions weakened.
e	Demonstrated both directions?	Yes. A/B'd pre-fix vs post-fix binaries: at max_ast_elements=100 the query passes on pre-fix eval and throws TOO_BIG_AST on post-fix eval (direct execution throws in both).
f	General, not a narrow patch?	The fix reuses the same visitor + ordering as executeQueryImpl, covering all global-WITH shapes (UNION/INTERSECT/EXCEPT branches, nested subqueries) rather than the single repro.
g	Generalizes across inputs?	N/A (ordering fix wiring the standard visitor; not an input-type-gated code path).
h	Backward compatible?	Yes. Gated by the existing enable_global_with_statement (default on); no format/setting-default change.
i	Invariants/contracts preserved?	Yes. Same visitor/ordering contract as executeQueryImpl; the inner query's own settings context drives expansion and limits.

Session id: cron:clickhouse-worker-slot-1:20260713-093100

[antaljanosbenjamin]

Could you add a new test instead of modifying existing one?

[groeneai]

Done in a755995. Moved the new regressions into a dedicated 04513_eval_table_function_ast_limits.sql; 04512_eval_table_function.sql/.reference are restored to their pre-PR content.

Pre-PR validation gate (click to expand)

#	Check	Result
a	Deterministic repro?	N/A (test-file reorganization, no code change)
b	Root cause explained?	N/A
c	Fix matches root cause?	N/A
d	Test intent preserved / new tests added?	Yes: the seven assertions are byte-identical, only relocated; 04512 is back to its pre-PR state
e	Demonstrated both directions?	Yes: all 7 assertions verified against the fixed binary (inner DISTINCT -> 1, ambiguous UNION -> EXPECTED_ALL_OR_DISTINCT, outer/inner max_ast_elements -> TOO_BIG_AST, max_ast_depth -> TOO_DEEP_AST, inner relax -> 55, global-WITH expansion -> TOO_BIG_AST)
f	Fix general, not a narrow patch?	N/A (no code bug)
g	Generalizes across inputs/types?	N/A
h	Backward compatible?	N/A
i	Invariants/contracts preserved?	Yes: new test is parallel-safe (no shared state, no no-parallel)

Session id: cron:clickhouse-worker-slot-1:20260713-113700

[groeneai]

CI finish ledger — a755995

Every failure below has an owner: a fixing PR (ours), or the private sync (exempt).

Check / test	Reason	Owner / fixing PR
Stateless (amd_asan_ubsan distributed/amd_debug/amd_msan WasmEdge/amd_tsan s3/arm_binary, sequential) / 02346_text_index_bug108519_qcc_skip_index	flaky (randomized materialize_statistics_on_insert=True)	#110260 (ours, merged 2026-07-13 15:05Z); this build predated it, merged master @ e68f5df to pick it up
AST fuzzer (amd_tsan) / Logical error 'or_argument_nodes.size() > 1' (STID 2508-3abc)	trunk (single-arg or() aborts LogicalExpressionOptimizer; unrelated to this PR's TableFunctionEval diff)	#107113 (ours, open)
Stateless (amd_tsan s3, sequential) / Scraping system tables	infra (minio_audit_logs dump timeout on s3 configs)	a fix task is moved to pending (investigating; fixing-PR link to follow here)
CH Inc sync	-	CH Inc sync (private, not actionable)

Session id: cron:our-pr-ci-monitor:20260713-183000

[groeneai]

Correction to the ledger above: the Scraping system tables / minio_audit_logs dump-timeout line has an existing fixing PR owner — #110099 (ours, open, info-only follow-up to merged #109837), not a new fix task.

[groeneai]

CI finish ledger — d9ed60b

Every failure below has an owner. Only CH Inc sync is exempt.

Check / test	Reason	Owner / fixing PR
Integration tests (amd_msan 7/8; amd_tsan 3/6) / test_replicated_database::test_replicated_table_structure_alter	flaky (trunk Replicated-DB DDL-recovery race; 230 PRs / 41 master, 14d)	#110030 (ours, open)
Sync	—	CH Inc sync (private, not actionable)

Session id: cron:our-pr-ci-monitor:20260714-093000

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	86.30%	86.40%	+0.10%
Functions	91.90%	91.90%	+0.00%
Branches	78.50%	78.50%	+0.00%

Changed lines: Changed C/C++ lines covered: 16/16 (100.00%) · Uncovered code

Full report · Diff report

[groeneai]

CI finish ledger — 442563c

Every failure below has an owner: a fixing PR (mine or external), or a full-effort fix task
whose fixing-PR link will be posted here when it opens. Only CH Inc sync is exempt.

Check / test	Reason	Owner / fixing PR
Stateless tests (amd_llvm_coverage, ParallelReplicas, s3 storage, parallel) / 01666_merge_tree_max_query_limit	the per-table concurrency throttle is keyed on query id, and secondary parallel-replica reads are sent with an empty query id, so the holder query is rejected with 202 TOO_MANY_SIMULTANEOUS_QUERIES by its own secondary reads	#112385 (mine, open)

This is a lane-reachability regression, not a defect in this PR: the test became reachable on the
ParallelReplicas lane when #111230 removed its parallel_replicas_blacklist.txt entry, and it
fails the same way on master. This PR's diff is src/TableFunctions/TableFunctionEval.cpp plus one
stateless test, which does not touch the concurrency throttle or parallel replicas.

Session id: cron:our-pr-ci-monitor:20260729-060000