Skip to content

Commit

Permalink
minor edits after self-review
Browse files Browse the repository at this point in the history 
Signed-off-by: Andres Taylor <[email protected]>
  • Loading branch information
@systay
systay committed Feb 6, 2024
1 parent 7fe7e6e commit 7754ade
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 51 deletions.
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/operators/SQL_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ func buildProjection(op *Projection, qb *queryBuilder) {

func buildApplyJoin(op *ApplyJoin, qb *queryBuilder) {
predicates := slice.Map(op.JoinPredicates.columns, func(jc applyJoinColumn) sqlparser.Expr {
// since we are adding these join predicates, we need to mark to broken up version of it as done
// since we are adding these join predicates, we need to mark to broken up version (RHSExpr) of it as done
err := qb.ctx.SkipJoinPredicates(jc.Original)
if err != nil {
panic(err)
Expand Down
6 changes: 5 additions & 1 deletion go/vt/vtgate/planbuilder/operators/projection_pushing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type (
}
)

// add will add a projection with a given alias
// add introduces a new projection with the specified alias to the projector.
func (p *projector) add(pe *ProjExpr, alias string) {
p.columns = append(p.columns, pe)
if alias != "" && slices.Index(p.columnAliases, alias) > -1 {
Expand All @@ -45,6 +45,7 @@ func (p *projector) add(pe *ProjExpr, alias string) {
p.columnAliases = append(p.columnAliases, alias)
}

// get finds or adds an expression in the projector, returning its SQL representation with the appropriate alias
func (p *projector) get(ctx *plancontext.PlanningContext, expr sqlparser.Expr) sqlparser.Expr {
for _, column := range p.columns {
if ctx.SemTable.EqualsExprWithDeps(expr, column.EvalExpr) {
Expand All @@ -71,6 +72,7 @@ func (p *projector) get(ctx *plancontext.PlanningContext, expr sqlparser.Expr) s
return out
}

// claimUnusedAlias generates a unique alias based on the provided expression, ensuring no duplication in the projector
func (p *projector) claimUnusedAlias(ae *sqlparser.AliasedExpr) string {
bare := ae.ColumnName()
alias := bare
Expand All @@ -83,6 +85,7 @@ func (p *projector) claimUnusedAlias(ae *sqlparser.AliasedExpr) string {
return alias
}

// tryPushProjection attempts to optimize a projection by pushing it down in the query plan
func tryPushProjection(
ctx *plancontext.PlanningContext,
p *Projection,
Expand Down Expand Up @@ -119,6 +122,7 @@ func tryPushProjection(
}
}

// pushProjectionThroughHashJoin optimizes projection operations within a hash join
func pushProjectionThroughHashJoin(ctx *plancontext.PlanningContext, p *Projection, hj *HashJoin) (Operator, *ApplyResult) {
cols := p.Columns.(AliasedProjections)
for _, col := range cols {
Expand Down
20 changes: 10 additions & 10 deletions go/vt/vtgate/planbuilder/operators/query_planning.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,16 +539,16 @@ func tryPushDistinct(in *Distinct) (Operator, *ApplyResult) {
in.PushedPerformance = true

return in, Rewrote("push down distinct under union")
// case JoinOp:
// src.SetLHS(&Distinct{Source: src.GetLHS()})
// src.SetRHS(&Distinct{Source: src.GetRHS()})
// in.PushedPerformance = true
//
// if in.Required {
// return in, Rewrote("push distinct under join - kept original")
// }
//
// return in.Source, Rewrote("push distinct under join")
case JoinOp:
src.SetLHS(&Distinct{Source: src.GetLHS()})
src.SetRHS(&Distinct{Source: src.GetRHS()})
in.PushedPerformance = true

if in.Required {
return in, Rewrote("push distinct under join - kept original")
}

return in.Source, Rewrote("push distinct under join")
case *Ordering:
in.Source = src.Source
return in, Rewrote("remove ordering under distinct")
Expand Down
10 changes: 6 additions & 4 deletions go/vt/vtgate/planbuilder/operators/route.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -615,16 +615,18 @@ type selectExpressions interface {
derivedName() string
}

// addColumnToInput adds a column to an operator without pushing it down.
// It will return a bool indicating whether the addition was successful or not,
// and an offset to where the column can be found
// addColumnToInput adds columns to an operator without pushing them down
func addMultipleColumnsToInput(
ctx *plancontext.PlanningContext,
operator Operator,
reuse bool,
addToGroupBy []bool,
exprs []*sqlparser.AliasedExpr,
) (derivedName string, projection Operator, found bool, offsets []int) {
) (derivedName string, // if we found a derived table, this will contain its name
projection Operator, // if an operator needed to be built, it will be returned here
found bool, // whether a matching op was found or not
offsets []int, // the offsets the expressions received
) {
switch op := operator.(type) {
case *SubQuery:
derivedName, src, added, offset := addMultipleColumnsToInput(ctx, op.Outer, reuse, addToGroupBy, exprs)
Expand Down
24 changes: 0 additions & 24 deletions go/vt/vtgate/planbuilder/plancontext/planning_context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,6 @@ func (ctx *PlanningContext) GetReservedArgumentFor(expr sqlparser.Expr) string {
return bvName
}

// GetArgumentFor retrieves or assigns a new argument name for a given expression.
// It uses a provided function to generate a new name if the expression doesn't
// have an existing argument name in the context.
func (ctx *PlanningContext) GetArgumentFor(expr sqlparser.Expr, f func() string) string {
for key, name := range ctx.ReservedArguments {
if ctx.SemTable.EqualsExpr(key, expr) {
return name
}
}
bvName := f()
ctx.ReservedArguments[expr] = bvName
return bvName
}

// ShouldSkip determines if a given expression should be ignored in the SQL output building.
// It checks against expressions that have been marked to be excluded from further processing.
func (ctx *PlanningContext) ShouldSkip(expr sqlparser.Expr) bool {
Expand Down Expand Up @@ -193,16 +179,6 @@ outer:
}
}

func (ctx *PlanningContext) CopyJoinPredicate(src, dest sqlparser.Expr) error {
fn := func(_ sqlparser.Expr, rhsExprs []sqlparser.Expr) {
ctx.joinPredicates[dest] = rhsExprs
}
if ctx.execOnJoinPredicateEqual(src, fn) {
return nil
}
return vterrors.VT13001("predicate does not exist: " + sqlparser.String(src))
}

func (ctx *PlanningContext) execOnJoinPredicateEqual(joinPred sqlparser.Expr, fn func(original sqlparser.Expr, rhsExprs []sqlparser.Expr)) bool {
for key, values := range ctx.joinPredicates {
if ctx.SemTable.EqualsExpr(joinPred, key) {
Expand Down
6 changes: 3 additions & 3 deletions go/vt/vtgate/planbuilder/testdata/from_cases.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4217,7 +4217,7 @@
"Sharded": true
},
"FieldQuery": "select id, col from (select id, col from `user` where 1 != 1) as u where 1 != 1",
"Query": "select id, col from (select id, col from `user`) as u",
"Query": "select distinct id, col from (select id, col from `user`) as u",
"Table": "`user`"
},
{
Expand Down Expand Up @@ -4310,7 +4310,7 @@
"Sharded": true
},
"FieldQuery": "select subquery_for_count.`m.id`, 1 from (select m.id as `m.id` from music as m where 1 != 1) as subquery_for_count where 1 != 1",
"Query": "select subquery_for_count.`m.id`, 1 from (select m.id as `m.id` from music as m) as subquery_for_count",
"Query": "select distinct subquery_for_count.`m.id`, 1 from (select m.id as `m.id` from music as m) as subquery_for_count",
"Table": "music"
},
{
Expand All @@ -4321,7 +4321,7 @@
"Sharded": true
},
"FieldQuery": "select subquery_for_count.user_id, subquery_for_count.`u.id`, weight_string(subquery_for_count.user_id) from (select u.user_id, u.id as `u.id` from `user` as u, user_extra as ue where 1 != 1) as subquery_for_count where 1 != 1",
"Query": "select subquery_for_count.user_id, subquery_for_count.`u.id`, weight_string(subquery_for_count.user_id) from (select u.user_id, u.id as `u.id` from `user` as u, user_extra as ue where u.id = :m_id and u.id = ue.user_id) as subquery_for_count",
"Query": "select distinct subquery_for_count.user_id, subquery_for_count.`u.id`, weight_string(subquery_for_count.user_id) from (select u.user_id, u.id as `u.id` from `user` as u, user_extra as ue where u.id = :m_id and u.id = ue.user_id) as subquery_for_count",
"Table": "`user`, user_extra",
"Values": [
":m_id"
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/planbuilder/testdata/postprocess_cases.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1720,7 +1720,7 @@
"Sharded": true
},
"FieldQuery": "select `user`.a, weight_string(`user`.a) from `user` where 1 != 1",
"Query": "select `user`.a, weight_string(`user`.a) from `user`",
"Query": "select distinct `user`.a, weight_string(`user`.a) from `user`",
"Table": "`user`"
},
{
Expand All @@ -1731,7 +1731,7 @@
"Sharded": true
},
"FieldQuery": "select 1 from user_extra where 1 != 1",
"Query": "select 1 from user_extra",
"Query": "select distinct 1 from user_extra",
"Table": "user_extra"
}
]
Expand Down
12 changes: 6 additions & 6 deletions go/vt/vtgate/planbuilder/testdata/union_cases.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@
"Sharded": true
},
"FieldQuery": "select `user`.id, `user`.`name`, weight_string(`user`.id), weight_string(`user`.`name`) from `user` where 1 != 1",
"Query": "select `user`.id, `user`.`name`, weight_string(`user`.id), weight_string(`user`.`name`) from `user`",
"Query": "select distinct `user`.id, `user`.`name`, weight_string(`user`.id), weight_string(`user`.`name`) from `user`",
"Table": "`user`"
},
{
Expand All @@ -587,7 +587,7 @@
"Sharded": true
},
"FieldQuery": "select 1 from user_extra where 1 != 1",
"Query": "select 1 from user_extra where user_extra.extra = 'asdf'",
"Query": "select distinct 1 from user_extra where user_extra.extra = 'asdf'",
"Table": "user_extra"
}
]
Expand Down Expand Up @@ -654,7 +654,7 @@
"Sharded": true
},
"FieldQuery": "select `user`.id, `user`.`name` from `user` where 1 != 1",
"Query": "select `user`.id, `user`.`name` from `user`",
"Query": "select distinct `user`.id, `user`.`name` from `user`",
"Table": "`user`"
},
{
Expand All @@ -665,7 +665,7 @@
"Sharded": true
},
"FieldQuery": "select 1 from user_extra where 1 != 1",
"Query": "select 1 from user_extra where user_extra.extra = 'asdf'",
"Query": "select distinct 1 from user_extra where user_extra.extra = 'asdf'",
"Table": "user_extra"
}
]
Expand Down Expand Up @@ -1333,7 +1333,7 @@
"Sharded": true
},
"FieldQuery": "select `user`.intcol, `user`.textcol2, weight_string(`user`.intcol), weight_string(`user`.textcol2) from `user` where 1 != 1",
"Query": "select `user`.intcol, `user`.textcol2, weight_string(`user`.intcol), weight_string(`user`.textcol2) from `user`",
"Query": "select distinct `user`.intcol, `user`.textcol2, weight_string(`user`.intcol), weight_string(`user`.textcol2) from `user`",
"Table": "`user`"
},
{
Expand All @@ -1344,7 +1344,7 @@
"Sharded": true
},
"FieldQuery": "select authoritative.col2, weight_string(authoritative.col2) from authoritative where 1 != 1",
"Query": "select authoritative.col2, weight_string(authoritative.col2) from authoritative",
"Query": "select distinct authoritative.col2, weight_string(authoritative.col2) from authoritative",
"Table": "authoritative"
}
]
Expand Down

0 comments on commit 7754ade

Please sign in to comment.