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.
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
Move subqueries to use the operator model #13750
Move subqueries to use the operator model #13750
Changes from all commits
a344c3a
98be673
ecb83d8
f359ed0
756e935
e0f00ce
f7d01e7
04ddc38
8aebcb9
9902d54
20e1340
0761a7a
7df2fdd
347a36a
337ec81
c988fad
71e19ad
eb06afd
3da93bc
60dddd5
7800ca9
f26eb69
24ab078
876f557
1bc2b4c
ca8e7c9
8f2e397
126a37f
8d7811f
8055ea7
f6b25b3
3856cb3
c39c5ce
6eb509e
52c5778
b0c0e7b
cee43e9
97868fd
45ef83c
f38f5dd
eb1d289
9dbd7b4
419dbfc
b1991cb
8213ea6
b455f80
407636e
0b36163
ca7e38e
87f9eb6
d4008e8
29cc45e
b2f5aea
5bcb900
868c6ba
c2e05c9
b21ab58
e8cb962
d2a4443
b823e7d
89c5431
8de4d9a
860f702
58fcb33
97023e8
5f90c4b
17402f9
741649a
df79570
e9f2315
16a31fc
5a9de19
252ee15
5520a31
386da80
972c1c3
5a9de9c
8864385
e70c5f2
5b74e3e
108a624
6d00c9a
1fb146a
d3d426e
49aefe5
54d6bd9
f0c76ee
155dc02
6cc5a6d
792ce30
a89e3d5
f9705b4
b756846
df4b8fa
ba8ea10
9ae46d1
bcb657d
e66336a
7ab4d8c
894823e
a104a2d
173ea54
eb23151
baeeabd
b804997
5f4b40d
b513c99
02afec8
826a3bb
77e4cb7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason for removing these test cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, they are not valid. they are not following the
ONLY_FULL_GROUP_BY
directive, but this test doesn't changesql_mode
on the connection, so the query fails.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this struct was a big part of the refactoring. Before this refactoring, we would replace all subqueries with this AST struct early on in the semantic analysis process, and use it during planning. All of that logic is now gone from semantic analysis and this struct is no longer needed.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why
first
field is required. Won't the first element ofnodes
otherwise be the first to run?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is to avoid accidental use of this method with no nodes to visit. We found one case where this was done by accident.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we then just check the length of nodes and panic/error on 0 length?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is that preferable to handling it like this?