-
Notifications
You must be signed in to change notification settings - Fork 8.8k
Apply AST size limits to the query generated by table function eval #110211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
groeneai
wants to merge
9
commits into
ClickHouse:master
Choose a base branch
from
groeneai:groeneai/eval-ast-size-limits
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+75
−3
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
727232d
Apply AST size limits to the query generated by table function eval
groeneai f502546
Resolve eval set operations with the inner query's own SETTINGS
groeneai efef3e9
eval: expand global WITH aliases before checking AST size limits
groeneai a755995
eval: move new AST-limit regressions to a dedicated test file
groeneai e68f5df
Merge master to pick up 02346 fix (#110260)
groeneai 7de01eb
Merge branch 'master' into groeneai/eval-ast-size-limits
alexey-milovidov d9ed60b
Add INTERSECT/EXCEPT inner-settings regression to 04513 eval test
groeneai 442563c
Merge branch 'master' into groeneai/eval-ast-size-limits
alexey-milovidov 2ad4949
Merge branch 'master' into groeneai/eval-ast-size-limits
alexey-milovidov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
4 changes: 4 additions & 0 deletions
4
tests/queries/0_stateless/04513_eval_table_function_ast_limits.reference
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| 1 | ||
| 2 | ||
| 3 | ||
| 55 |
35 changes: 35 additions & 0 deletions
35
tests/queries/0_stateless/04513_eval_table_function_ast_limits.sql
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| -- Tags: no-old-analyzer | ||
| -- no-old-analyzer: `eval` requires the analyzer. | ||
|
|
||
| -- The generated query is analyzed with the generated query's own SETTINGS, so its set operation modes | ||
| -- and AST size limits behave the same as when the query is executed directly. | ||
| -- See https://github.com/ClickHouse/ClickHouse/pull/110211 | ||
|
|
||
| SET allow_experimental_eval_table_function = 1; | ||
|
|
||
| -- The set operation modes are resolved from the generated query's own SETTINGS, so an inner | ||
| -- `union_default_mode = 'DISTINCT'` normalizes an ambiguous UNION the same way it would when the | ||
| -- query is executed directly, even though the outer default would reject it. | ||
| SELECT count() FROM eval('SELECT 1 AS n UNION SELECT 1 AS n SETTINGS union_default_mode = ''DISTINCT'''); | ||
|
clickhouse-gh[bot] marked this conversation as resolved.
|
||
| SELECT count() FROM eval('SELECT 1 AS n UNION SELECT 1 AS n'); -- { serverError EXPECTED_ALL_OR_DISTINCT } | ||
|
|
||
| -- INTERSECT / EXCEPT default modes are likewise resolved from the generated query's own SETTINGS. | ||
| -- These modes live in separate branches of `SelectIntersectExceptQueryVisitor`, so the inner | ||
| -- `intersect_default_mode` / `except_default_mode = 'ALL'` must win over the outer `'DISTINCT'`, | ||
| -- keeping duplicates exactly as when the query is executed directly. INTERSECT ALL of [1,1,2] and | ||
| -- [1,1,3] keeps two 1s; EXCEPT ALL of [1,1,1,2] and [1] keeps 1,1,2. | ||
| SELECT count() FROM eval('SELECT arrayJoin([1, 1, 2]) AS n INTERSECT SELECT arrayJoin([1, 1, 3]) AS n SETTINGS intersect_default_mode = ''ALL''') SETTINGS intersect_default_mode = 'DISTINCT'; | ||
| SELECT count() FROM eval('SELECT arrayJoin([1, 1, 1, 2]) AS n EXCEPT SELECT arrayJoin([1]) AS n SETTINGS except_default_mode = ''ALL''') SETTINGS except_default_mode = 'DISTINCT'; | ||
|
|
||
| -- The AST size limits `max_ast_elements` / `max_ast_depth` apply to the generated query, same as when | ||
| -- it is executed directly, so a tiny outer query cannot smuggle a huge or deep AST past them. | ||
| SELECT count() FROM eval('SELECT 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10') SETTINGS max_ast_elements = 30; -- { serverError TOO_BIG_AST } | ||
| SELECT count() FROM eval('SELECT (((((1)))))') SETTINGS max_ast_depth = 3; -- { serverError TOO_DEEP_AST } | ||
| -- The limits are read after the generated query's own SETTINGS are applied, so an inner SETTINGS clause | ||
| -- controls its own AST size limits, both to tighten and to relax them. | ||
| SELECT count() FROM eval('SELECT 1 + 2 + 3 + 4 + 5 SETTINGS max_ast_elements = 5'); -- { serverError TOO_BIG_AST } | ||
| SELECT * FROM eval('SELECT 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 AS big SETTINGS max_ast_elements = 100000') SETTINGS max_ast_elements = 30; | ||
| -- The size limits are checked after the global `WITH` aliases are expanded (as in a direct query), so a | ||
| -- global CTE that stays small before expansion but grows past `max_ast_elements` once it is inlined into | ||
| -- every UNION branch is rejected, instead of slipping through the pre-expansion check. | ||
| SELECT count() FROM eval('WITH 1+2+3+4+5+6+7+8+9+10+11+12+13+14+15 AS big SELECT big UNION ALL SELECT big UNION ALL SELECT big UNION ALL SELECT big UNION ALL SELECT big UNION ALL SELECT big UNION ALL SELECT big UNION ALL SELECT big SETTINGS max_ast_elements = 100'); -- { serverError TOO_BIG_AST } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.