Skip to content

Commit

Permalink
refactor: address review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Andres Taylor <[email protected]>
  • Loading branch information
systay committed Apr 16, 2024
1 parent 87c7426 commit 8eaf17a
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 11 deletions.
1 change: 0 additions & 1 deletion go/mysql/sqlerror/sql_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ var stateToMysqlCode = map[vterrors.State]mysqlCode{
vterrors.KillDeniedError: {num: ERKillDenied, state: SSUnknownSQLState},
vterrors.BadNullError: {num: ERBadNullError, state: SSConstraintViolation},
vterrors.InvalidGroupFuncUse: {num: ERInvalidGroupFuncUse, state: SSUnknownSQLState},
vterrors.AggregateMustPushDown: {num: ERNotSupportedYet, state: SSUnknownSQLState},
}

func getStateToMySQLState(state vterrors.State) mysqlCode {
Expand Down
1 change: 0 additions & 1 deletion go/vt/vterrors/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ var (
VT03030 = errorWithState("VT03030", vtrpcpb.Code_INVALID_ARGUMENT, WrongValueCountOnRow, "lookup column count does not match value count with the row (columns, count): (%v, %d)", "The number of columns you want to insert do not match the number of columns of your SELECT query.")
VT03031 = errorWithoutState("VT03031", vtrpcpb.Code_INVALID_ARGUMENT, "EXPLAIN is only supported for single keyspace", "EXPLAIN has to be sent down as a single query to the underlying MySQL, and this is not possible if it uses tables from multiple keyspaces")
VT03032 = errorWithState("VT03032", vtrpcpb.Code_INVALID_ARGUMENT, NonUpdateableTable, "the target table %s of the UPDATE is not updatable", "You cannot update a table that is not a real MySQL table.")
VT03033 = errorWithState("VT03033", vtrpcpb.Code_INVALID_ARGUMENT, AggregateMustPushDown, "aggregate user-defined function %s must be pushed down to mysql", "The aggregate user-defined function must be pushed down to mysql and can't be evaluated on the vtgate. The query contains aggregation that can't be fully pushed down to MySQL.")

VT05001 = errorWithState("VT05001", vtrpcpb.Code_NOT_FOUND, DbDropExists, "cannot drop database '%s'; database does not exists", "The given database does not exist; Vitess cannot drop it.")
VT05002 = errorWithState("VT05002", vtrpcpb.Code_NOT_FOUND, BadDb, "cannot alter database '%s'; unknown database", "The given database does not exist; Vitess cannot alter it.")
Expand Down
1 change: 0 additions & 1 deletion go/vt/vterrors/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const (
WrongArguments
BadNullError
InvalidGroupFuncUse
AggregateMustPushDown

// failed precondition
NoDB
Expand Down
7 changes: 4 additions & 3 deletions go/vt/vtgate/planbuilder/operators/offset_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ func addColumnsToInput(ctx *plancontext.PlanningContext, root Operator) Operator
for _, aggr := range aggrOp.Aggregations {
if aggr.OpCode == opcode.AggregateUDF {
// we don't support UDFs in aggregation if it's still above a route
panic(vterrors.VT03033(sqlparser.String(aggr.Original.Expr)))
message := fmt.Sprintf("Aggregate UDF %s must be pushed down to MySQL", sqlparser.String(aggr.Original.Expr))
panic(vterrors.VT12001(message))
}
}
return in, NoRewrite
Expand All @@ -154,8 +155,8 @@ func addColumnsToInput(ctx *plancontext.PlanningContext, root Operator) Operator
return TopDown(root, TableID, visitor, stopAtRoute)
}

// pullDistinctFromUNION will pull out the distinct from a union operator
func pullDistinctFromUNION(_ *plancontext.PlanningContext, root Operator) Operator {
// isolateDistinctFromUnion will pull out the distinct from a union operator
func isolateDistinctFromUnion(_ *plancontext.PlanningContext, root Operator) Operator {
visitor := func(in Operator, _ semantics.TableSet, isRoot bool) (Operator, *ApplyResult) {
union, ok := in.(*Union)
if !ok || !union.distinct {
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/operators/phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (p Phase) shouldRun(s semantics.QuerySignature) bool {
func (p Phase) act(ctx *plancontext.PlanningContext, op Operator) Operator {
switch p {
case pullDistinctFromUnion:
return pullDistinctFromUNION(ctx, op)
return isolateDistinctFromUnion(ctx, op)
case delegateAggregation:
return enableDelegateAggregation(ctx, op)
case addAggrOrdering:
Expand Down
4 changes: 1 addition & 3 deletions go/vt/vtgate/planbuilder/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,7 @@ func TestOtherPlanningFromFile(t *testing.T) {

func loadSchema(t testing.TB, filename string, setCollation bool) *vindexes.VSchema {
formal, err := vindexes.LoadFormal(locateFile(filename))
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
vschema := vindexes.BuildVSchema(formal, sqlparser.NewTestParser())
require.NoError(t, err)
for _, ks := range vschema.Keyspaces {
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/vindexes/vschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ func (vschema *VSchema) AddView(ksname, viewName, query string, parser *sqlparse
return nil
}

// AddTable adds a table to an existing keyspace in the VSchema.
// AddUDF adds a UDF to an existing keyspace in the VSchema.
// It's only used from tests.
func (vschema *VSchema) AddUDF(ksname, udfName string) error {
ks, ok := vschema.Keyspaces[ksname]
Expand Down

0 comments on commit 8eaf17a

Please sign in to comment.