adapter: Inherit allow_subqueries from outer context in sql_impl #34892
+38
−10
Conversation
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
Contributor
Author
|
The CI fail is caused by the PR; I'm investigating. Edit: Fixed in the second commit. |
ggevay
force-pushed
the
sql_impl-ExprContext
branch
from
e9c7987 to
6a790c3
Compare
def-
approved these changes
Feb 5, 2026
Contributor
def-
left a comment
def-
left a comment
There was a problem hiding this comment.
Definitely fixes the bug, thanks for adding a test
Contributor
|
Any chance to merge this? Currently blocking introducing SQLancer++ testing |
mtabebe
reviewed
Feb 11, 2026
| # TODO: Bad error msg, because we handle ExprContext names badly when inner contexts are created by `with_name`. See | ||
| # https://github.com/MaterializeInc/database-issues/issues/10074 | ||
| statement error db error: ERROR: static function definition \(or its outer context 'mz_internal\.mz_role_oid'\) does not allow subqueries | ||
| UPDATE t0 SET c1=HAS_SECRET_PRIVILEGE(t0.c1, '30 minutes') WHERE (t0.c1 BETWEEN t0.c1 AND t0.c1); |
Contributor
There was a problem hiding this comment.
I think it would be good to have a basic test that shows that the subquery is still allowed (e.g., outside the context of a set).
SELECT HAS_SECRET_PRIVILEGE(t0.c1, '30 minutes') FROM t0 LIMIT 0;
Additionally, maybe it would be good to have some examples of deeper nesting?
mtabebe
reviewed
Feb 11, 2026
Contributor
mtabebe
left a comment
mtabebe
left a comment
There was a problem hiding this comment.
My reading of this makes sense, just some small test suggestions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes https://github.com/MaterializeInc/database-issues/issues/10067. (And unblocks #34881)
The problem was that we were simply always statically allowing subqueries in the
ExprContextinsql_impl. This meant that we allowed subqueries there even if an outer context (such as theSETpart of anUPDATE) didn't allow it. This PR fixes this by passing in the outer context, and inheriting whether that allows subqueries.(Unfortunately, the error msg didn't come out nice, see the issue that is linked from the slt.)
Edit: The second commit tweaks how we handle errors in
generate_column_casts. The issue was that that function relied on the buggy behavior ofsql_implthat allowed subqueries without regard to the outer context, so that it can catch subqueries specifically bylower_uncorrelated. So, I've now had to explicitly make the outer context allow subqueries, so thatlower_uncorrelatedcan keep catching it.