chore: refactor and some additional comments and renames

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
vitessio · Jun 12, 2024 · 973f29f · 973f29f
1 parent 3f7b825
commit 973f29f
Show file tree

Hide file tree

Showing 13 changed files with 22 additions and 36 deletions.
diff --git a/go/vt/vtgate/planbuilder/fuzz.go b/go/vt/vtgate/planbuilder/fuzz.go
@@ -20,12 +20,12 @@ import (
  "sync"
  "testing"
 
+ fuzz "github.com/AdaLogics/go-fuzz-headers"
+
  "vitess.io/vitess/go/json2"
  "vitess.io/vitess/go/sqltypes"
  vschemapb "vitess.io/vitess/go/vt/proto/vschema"
  "vitess.io/vitess/go/vt/vtgate/vindexes"
-
- fuzz "github.com/AdaLogics/go-fuzz-headers"
 )
 
 var initter sync.Once

diff --git a/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go b/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go
@@ -89,7 +89,7 @@ func reachedPhase(ctx *plancontext.PlanningContext, p Phase) bool {
 // Any columns that are needed to evaluate the subquery needs to be added as
 // grouping columns to the aggregation being pushed down, and then after the
 // subquery evaluation we are free to reassemble the total aggregation values.
-// This is very similar to how we push aggregation through an apply -join.
+// This is very similar to how we push aggregation through an apply-join.
 func pushAggregationThroughSubquery(
  ctx *plancontext.PlanningContext,
  rootAggr *Aggregator,

diff --git a/go/vt/vtgate/planbuilder/operators/query_planning.go b/go/vt/vtgate/planbuilder/operators/query_planning.go
@@ -20,12 +20,10 @@ import (
  "fmt"
  "io"
 
- "vitess.io/vitess/go/vt/vtgate/engine"
-
+ "vitess.io/vitess/go/vt/sqlparser"
  "vitess.io/vitess/go/vt/vterrors"
+ "vitess.io/vitess/go/vt/vtgate/engine"
  "vitess.io/vitess/go/vt/vtgate/evalengine"
-
- "vitess.io/vitess/go/vt/sqlparser"
  "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
  "vitess.io/vitess/go/vt/vtgate/semantics"
 )
@@ -713,7 +711,7 @@ func colNamesAlign(expected, actual sqlparser.SelectExprs) bool {
  case *sqlparser.StarExpr:
  actualStar, isStar := actual[i].(*sqlparser.StarExpr)
  if !isStar {
- panic(vterrors.VT13001("this should not happen"))
+ panic(vterrors.VT13001(fmt.Sprintf("star expression is expected here, found: %T", actual[i])))
  }
  if !sqlparser.Equals.RefOfStarExpr(se, actualStar) {
  return false

diff --git a/go/vt/vtgate/planbuilder/operators/queryprojection.go b/go/vt/vtgate/planbuilder/operators/queryprojection.go
@@ -269,7 +269,7 @@ func (qp *QueryProjection) addOrderBy(ctx *plancontext.PlanningContext, orderBy
  canPushSorting := true
  es := &expressionSet{}
  for _, order := range orderBy {
- if notInterestingToOrderBy(ctx, order.Expr) {
+ if canIgnoreOrdering(ctx, order.Expr) {
  continue
  }
  if !es.add(ctx, order.Expr) {
@@ -283,7 +283,8 @@ func (qp *QueryProjection) addOrderBy(ctx *plancontext.PlanningContext, orderBy
  }
 }
 
-func notInterestingToOrderBy(ctx *plancontext.PlanningContext, expr sqlparser.Expr) bool {
+// canIgnoreOrdering returns true if the ordering expression has no effect on the result.
+func canIgnoreOrdering(ctx *plancontext.PlanningContext, expr sqlparser.Expr) bool {
  switch expr.(type) {
  case *sqlparser.NullVal, *sqlparser.Literal, *sqlparser.Argument:
  return true

diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go
@@ -28,10 +28,9 @@ import (
  "strings"
  "testing"
 
- "github.com/stretchr/testify/suite"
-
  "github.com/nsf/jsondiff"
  "github.com/stretchr/testify/require"
+ "github.com/stretchr/testify/suite"
 
  "vitess.io/vitess/go/sqltypes"
  "vitess.io/vitess/go/test/utils"

diff --git a/go/vt/vtgate/planbuilder/planner_test.go b/go/vt/vtgate/planbuilder/planner_test.go
@@ -19,11 +19,10 @@ package planbuilder
 import (
  "testing"
 
- "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
-
  "github.com/stretchr/testify/require"
 
  "vitess.io/vitess/go/vt/sqlparser"
+ "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
  "vitess.io/vitess/go/vt/vtgate/semantics"
  "vitess.io/vitess/go/vt/vtgate/vindexes"
 )

diff --git a/go/vt/vtgate/planbuilder/predicate_rewrite_test.go b/go/vt/vtgate/planbuilder/predicate_rewrite_test.go
@@ -27,7 +27,6 @@ import (
  "github.com/stretchr/testify/require"
 
  "vitess.io/vitess/go/mysql/collations"
-
  "vitess.io/vitess/go/sqltypes"
  "vitess.io/vitess/go/vt/sqlparser"
  "vitess.io/vitess/go/vt/vtenv"

diff --git a/go/vt/vtgate/planbuilder/rewrite_test.go b/go/vt/vtgate/planbuilder/rewrite_test.go
@@ -19,12 +19,11 @@ package planbuilder
 import (
  "testing"
 
- "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
-
  "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
 
  "vitess.io/vitess/go/vt/sqlparser"
+ "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
  "vitess.io/vitess/go/vt/vtgate/semantics"
 )
 

diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go
@@ -21,18 +21,14 @@ import (
  "strconv"
  "strings"
 
- "vitess.io/vitess/go/vt/sysvars"
- "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
-
- "vitess.io/vitess/go/vt/vtgate/evalengine"
-
- "vitess.io/vitess/go/vt/vtgate/vindexes"
-
- "vitess.io/vitess/go/vt/vterrors"
-
  "vitess.io/vitess/go/vt/key"
  "vitess.io/vitess/go/vt/sqlparser"
+ "vitess.io/vitess/go/vt/sysvars"
+ "vitess.io/vitess/go/vt/vterrors"
  "vitess.io/vitess/go/vt/vtgate/engine"
+ "vitess.io/vitess/go/vt/vtgate/evalengine"
+ "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
+ "vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 
 type (

diff --git a/go/vt/vtgate/planbuilder/show_test.go b/go/vt/vtgate/planbuilder/show_test.go
@@ -21,15 +21,13 @@ import (
  "fmt"
  "testing"
 
- "vitess.io/vitess/go/test/vschemawrapper"
- "vitess.io/vitess/go/vt/vtenv"
-
  "github.com/stretchr/testify/require"
 
  "vitess.io/vitess/go/mysql/collations"
-
  "vitess.io/vitess/go/sqltypes"
+ "vitess.io/vitess/go/test/vschemawrapper"
  "vitess.io/vitess/go/vt/sqlparser"
+ "vitess.io/vitess/go/vt/vtenv"
  "vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 

diff --git a/go/vt/vtgate/planbuilder/vexplain.go b/go/vt/vtgate/planbuilder/vexplain.go
@@ -20,8 +20,6 @@ import (
  "context"
  "encoding/json"
 
- "vitess.io/vitess/go/vt/vtgate/vindexes"
-
  "vitess.io/vitess/go/sqltypes"
  "vitess.io/vitess/go/vt/key"
  querypb "vitess.io/vitess/go/vt/proto/query"
@@ -31,6 +29,7 @@ import (
  "vitess.io/vitess/go/vt/vtgate/engine"
  "vitess.io/vitess/go/vt/vtgate/planbuilder/operators"
  "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
+ "vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 
 func buildVExplainPlan(ctx context.Context, vexplainStmt *sqlparser.VExplainStmt, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, enableOnlineDDL, enableDirectDDL bool) (*planResult, error) {

diff --git a/go/vt/vtgate/planbuilder/vindex_func.go b/go/vt/vtgate/planbuilder/vindex_func.go
@@ -20,10 +20,9 @@ import (
  "fmt"
 
  "vitess.io/vitess/go/mysql/collations"
- "vitess.io/vitess/go/vt/vterrors"
-
  querypb "vitess.io/vitess/go/vt/proto/query"
  "vitess.io/vitess/go/vt/sqlparser"
+ "vitess.io/vitess/go/vt/vterrors"
  "vitess.io/vitess/go/vt/vtgate/engine"
 )
 

diff --git a/go/vt/vtgate/planbuilder/vstream.go b/go/vt/vtgate/planbuilder/vstream.go
@@ -20,13 +20,12 @@ import (
  "strconv"
  "strings"
 
- "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
-
  "vitess.io/vitess/go/vt/key"
  topodatapb "vitess.io/vitess/go/vt/proto/topodata"
  "vitess.io/vitess/go/vt/sqlparser"
  "vitess.io/vitess/go/vt/vterrors"
  "vitess.io/vitess/go/vt/vtgate/engine"
+ "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 )
 
 const defaultLimit = 100