Skip to content
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

Gen4: subquery refactor #8974

Merged
merged 19 commits into from
Oct 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a901e28
Addition of the ExtractedSubquery AST node to make easy subquery plan…
frouioui Oct 8, 2021
0560feb
allow planComparison to exit early when we know we will plan a Select…
systay Oct 11, 2021
36141d2
Update plan_test expectations
systay Oct 11, 2021
efd95b2
Don't fail dependency logic on slices
systay Oct 11, 2021
5497971
support for ::__vals argument when using IN subqueries
frouioui Oct 11, 2021
0573e46
fix impossible IN operation
frouioui Oct 11, 2021
148513e
using rewritten subquery to search new vindexes
frouioui Oct 11, 2021
41541b0
fix issue with IN subqueries becoming scatter when merging them
frouioui Oct 11, 2021
2483b11
changed the way we calculate dependencies for ExtractedSubqueries
frouioui Oct 11, 2021
f31d4c3
add ExtractedSubquery to the precedence logic
systay Oct 11, 2021
e12a286
Updated plan tests with the new ExtractedSubquery precedence logic
frouioui Oct 11, 2021
dc15f61
Fixed ExtractedSuqbuery issue with routed tables
frouioui Oct 11, 2021
1f2bb8d
fixed issue when dealing with two subqueries that are equal
frouioui Oct 12, 2021
96a780d
fixed issue with ExtractedSubquery's Original not mimicking the chang…
frouioui Oct 12, 2021
90d924c
applied format and import
frouioui Oct 12, 2021
ee3a800
addition of unit test for semtable's subquery mapping
frouioui Oct 12, 2021
986cb67
allow any ComparisonOperator for comparison subqueries
frouioui Oct 12, 2021
cfa5705
correction of a few nits and addition of a new plan test
frouioui Oct 12, 2021
24ad27c
added pullout vars to the plan description
harshit-gangal Oct 12, 2021
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
13 changes: 13 additions & 0 deletions go/vt/sqlparser/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -1963,6 +1963,18 @@ type (
Name ColIdent
Fsp Expr // fractional seconds precision, integer from 0 to 6
}

// ExtractedSubquery is a subquery that has been extracted from the original AST
// This is a struct that the parser will never produce - it's written and read by the gen4 planner
ExtractedSubquery struct {
Original Expr // original expression that was replaced by this ExtractedSubquery
ArgName string
HasValuesArg string
OpCode int // this should really be engine.PulloutOpCode, but we cannot depend on engine :(
Copy link
Member

Choose a reason for hiding this comment

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

Should we move the opcode here then? or some other common package?

Subquery SelectStatement
OtherSide Expr // represents the side of the comparison, this field will be nil if Original is not a comparison
frouioui marked this conversation as resolved.
Show resolved Hide resolved
NeedsRewrite bool // tells whether we need to rewrite this subquery to Original or not
}
)

// iExpr ensures that only expressions nodes can be assigned to a Expr
Expand Down Expand Up @@ -1997,6 +2009,7 @@ func (*ConvertUsingExpr) iExpr() {}
func (*MatchExpr) iExpr() {}
func (*GroupConcatExpr) iExpr() {}
func (*Default) iExpr() {}
func (*ExtractedSubquery) iExpr() {}

// Exprs represents a list of value expressions.
// It's not a valid expression because it's not parenthesized.
Expand Down
16 changes: 16 additions & 0 deletions go/vt/sqlparser/ast_clone.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 29 additions & 0 deletions go/vt/sqlparser/ast_equals.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions go/vt/sqlparser/ast_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -1720,3 +1720,33 @@ func (node *RenameTable) Format(buf *TrackedBuffer) {
prefix = ", "
}
}

func (node *ExtractedSubquery) Format(buf *TrackedBuffer) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing comment, and we should not create the new expressions here, we should create them when we create this struct, I think

Copy link
Member

Choose a reason for hiding this comment

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

as the new struct is also an SQLNode, we can keep the Format function here.

switch original := node.Original.(type) {
case *ExistsExpr:
buf.astPrintf(node, "%v", NewArgument(node.ArgName))
case *ComparisonExpr:
// other_side = :__sq
cmp := &ComparisonExpr{
Left: node.OtherSide,
Right: NewArgument(node.ArgName),
Operator: original.Operator,
}
var expr Expr = cmp
switch original.Operator {
case InOp:
// :__sq_has_values = 1 and other_side in ::__sq
cmp.Right = NewListArg(node.ArgName)
hasValue := &ComparisonExpr{Left: NewArgument(node.HasValuesArg), Right: NewIntLiteral("1"), Operator: EqualOp}
expr = AndExpressions(hasValue, cmp)
case NotInOp:
// :__sq_has_values = 0 or other_side not in ::__sq
cmp.Right = NewListArg(node.ArgName)
hasValue := &ComparisonExpr{Left: NewArgument(node.HasValuesArg), Right: NewIntLiteral("0"), Operator: EqualOp}
expr = OrExpressions(hasValue, cmp)
}
buf.astPrintf(node, "%v", expr)
case *Subquery:
buf.astPrintf(node, "%v", NewArgument(node.ArgName))
}
}
30 changes: 30 additions & 0 deletions go/vt/sqlparser/ast_format_fast.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 41 additions & 0 deletions go/vt/sqlparser/ast_rewrite.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions go/vt/sqlparser/ast_visit.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions go/vt/sqlparser/cached_size.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions go/vt/sqlparser/precedence.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ func precedenceFor(in Expr) Precendence {
}
case *IntervalExpr:
return P1
case *ExtractedSubquery:
return precedenceFor(node.Original)
}

return Syntactic
Expand Down
23 changes: 23 additions & 0 deletions go/vt/sqlparser/precedence_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -65,6 +66,28 @@ func TestAndOrPrecedence(t *testing.T) {
}
}

func TestNotInSubqueryPrecedence(t *testing.T) {
tree, err := Parse("select * from a where not id in (select 42)")
require.NoError(t, err)
not := tree.(*Select).Where.Expr.(*NotExpr)
cmp := not.Expr.(*ComparisonExpr)
subq := cmp.Right.(*Subquery)
fmt.Println(subq)

extracted := &ExtractedSubquery{
Original: cmp,
ArgName: "arg1",
HasValuesArg: "has_values1",
OpCode: 1,
Subquery: subq.Select,
OtherSide: cmp.Left,
}
not.Expr = extracted

output := readable(not)
assert.Equal(t, "not (:has_values1 = 1 and id in ::arg1)", output)
}

func TestPlusStarPrecedence(t *testing.T) {
validSQL := []struct {
input string
Expand Down
2 changes: 1 addition & 1 deletion go/vt/sqlparser/tracked_buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (buf *TrackedBuffer) formatter(node SQLNode) {
}
}

//needParens says if we need a parenthesis
// needParens says if we need a parenthesis
// op is the operator we are printing
// val is the value we are checking if we need parens around or not
// left let's us know if the value is on the lhs or rhs of the operator
Expand Down
12 changes: 12 additions & 0 deletions go/vt/vtgate/engine/pullout_subquery.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,21 @@ func (ps *PulloutSubquery) execSubquery(vcursor VCursor, bindVars map[string]*qu
}

func (ps *PulloutSubquery) description() PrimitiveDescription {
other := map[string]interface{}{}
var pulloutVars []string
if ps.HasValues != "" {
pulloutVars = append(pulloutVars, ps.HasValues)
}
if ps.SubqueryResult != "" {
pulloutVars = append(pulloutVars, ps.SubqueryResult)
}
if len(pulloutVars) > 0 {
other["PulloutVars"] = pulloutVars
}
return PrimitiveDescription{
OperatorType: "Subquery",
Variant: ps.Opcode.String(),
Other: other,
}
}

Expand Down
Loading