Skip to content

Set parsed comments in operator for subqueries #18369

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.

Sign up for GitHub

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

Merged
systay merged 7 commits into from
Jul 1, 2025
Merged

Set parsed comments in operator for subqueries #18369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
83db176
Set parsed comments in op for subqueries
davidpiegza Jun 17, 2025
7bdb984
Fix formatting
davidpiegza Jun 17, 2025
44897a2
Only set MAX_EXECUTION_TIME comment in op
davidpiegza Jun 20, 2025
c6131af
Revert "Only set MAX_EXECUTION_TIME comment in op"
davidpiegza Jun 24, 2025
5859eee
Add test case for subquery with a comment
davidpiegza Jun 24, 2025
b527cf4
Update test query in select_cases
davidpiegza Jun 24, 2025
729549c
Update test query in select_cases.json
davidpiegza Jun 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions go/vt/vtgate/planbuilder/operators/subquery_planning.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,9 @@ func tryMergeSubqueryWithOuter(ctx *plancontext.PlanningContext, subQuery *SubQu
if !subQuery.IsArgument {
op.Source = newFilter(outer.Source, subQuery.Original)
}
if outer.Comments != nil {
op.Comments = outer.Comments
}
ctx.MergedSubqueries = append(ctx.MergedSubqueries, subQuery.originalSubquery)
return op, Rewrote("merged subquery with outer")
}
Expand Down
68 changes: 68 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/select_cases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,74 @@
},
"skip_e2e": true
},
{
"comment": "Comments with subquery",
"query": "select /* comment */ user.col from user where id IN (select id from user where id > 1 and id < 10)",
Copy link
Collaborator

@systay systay Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add a test for a plan that is not merged into a single route. we want the comment to be there for the subquery if it's sent separately. this test is not showing how we handle comments on the subquery

Copy link
Contributor Author

@davidpiegza davidpiegza Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@systay Thanks for the review! I added another test for a plan with a "complex" expression.

"plan": {
"Type": "Scatter",
"QueryType": "SELECT",
"Original": "select /* comment */ user.col from user where id IN (select id from user where id > 1 and id < 10)",
"Instructions": {
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select `user`.col from `user` where 1 != 1",
"Query": "select /* comment */ `user`.col from `user` where id in (select id from `user` where id > 1 and id < 10)"
},
"TablesUsed": [
"user.user"
]
},
"skip_e2e": true
},
{
"comment": "Comments with subquery not merged into a single route",
"query": "select /* comment */ user.col from user where foo IN (select id from user where id > 1 and id < 10)",
"plan": {
"Type": "Complex",
"QueryType": "SELECT",
"Original": "select /* comment */ user.col from user where foo IN (select id from user where id > 1 and id < 10)",
"Instructions": {
"OperatorType": "UncorrelatedSubquery",
"Variant": "PulloutIn",
"PulloutVars": [
"__sq_has_values",
"__sq1"
],
"Inputs": [
{
"InputName": "SubQuery",
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select /* comment */ id from `user` where id > 1 and id < 10"
},
{
"InputName": "Outer",
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select `user`.col from `user` where 1 != 1",
"Query": "select /* comment */ `user`.col from `user` where :__sq_has_values and foo in ::__sq1"
}
]
},
"TablesUsed": [
"user.user"
]
},
"skip_e2e": true
},
{
"comment": "for update",
"query": "select user.col from user join user_extra for update",
Expand Down
Loading