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: Fail all queries not handled well by gen4 #8359

Merged
merged 15 commits into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion go/vt/sqlparser/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ type (
Distinct bool
StraightJoinHint bool
SQLCalcFoundRows bool
From []TableExpr
Comments Comments
SelectExprs SelectExprs
From TableExprs
Where *Where
GroupBy GroupBy
Having *Where
Expand Down
11 changes: 10 additions & 1 deletion go/vt/sqlparser/ast_clone.go

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

15 changes: 14 additions & 1 deletion go/vt/sqlparser/ast_equals.go

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

13 changes: 10 additions & 3 deletions go/vt/sqlparser/ast_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,16 @@ func (node *Select) Format(buf *TrackedBuffer) {
buf.WriteString(SQLCalcFoundRowsStr)
}

buf.astPrintf(node, "%v from %v%v%v%v%v%v%s%v",
node.SelectExprs,
node.From, node.Where,
buf.astPrintf(node, "%v from ", node.SelectExprs)

prefix := ""
for _, expr := range node.From {
buf.astPrintf(node, "%s%v", prefix, expr)
prefix = ", "
}

buf.astPrintf(node, "%v%v%v%v%v%s%v",
node.Where,
node.GroupBy, node.Having, node.OrderBy,
node.Limit, node.Lock.ToString(), node.Into)
}
Expand Down
7 changes: 6 additions & 1 deletion go/vt/sqlparser/ast_format_fast.go

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

12 changes: 12 additions & 0 deletions go/vt/sqlparser/ast_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1339,3 +1339,15 @@ func handleUnaryMinus(expr Expr) Expr {
func encodeSQLString(val string) string {
return sqltypes.EncodeStringSQL(val)
}

// ToString prints the list of table expressions as a string
// To be used as an alternate for String for []TableExpr
func ToString(exprs []TableExpr) string {
buf := NewTrackedBuffer(nil)
prefix := ""
for _, expr := range exprs {
buf.astPrintf(nil, "%s%v", prefix, expr)
prefix = ", "
}
return buf.String()
}
14 changes: 9 additions & 5 deletions go/vt/sqlparser/ast_rewrite.go

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

8 changes: 5 additions & 3 deletions go/vt/sqlparser/ast_visit.go

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

18 changes: 9 additions & 9 deletions go/vt/sqlparser/cached_size.go

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

8 changes: 7 additions & 1 deletion go/vt/sqlparser/impossible_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ package sqlparser
func FormatImpossibleQuery(buf *TrackedBuffer, node SQLNode) {
switch node := node.(type) {
case *Select:
buf.Myprintf("select %v from %v where 1 != 1", node.SelectExprs, node.From)
buf.Myprintf("select %v from ", node.SelectExprs)
var prefix string
for _, n := range node.From {
buf.Myprintf("%s%v", prefix, n)
prefix = ", "
}
buf.Myprintf(" where 1 != 1")
if node.GroupBy != nil {
node.GroupBy.Format(buf)
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtctl/workflow/vexec/vexec.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func extractTableName(stmt sqlparser.Statement) (string, error) {
case *sqlparser.Insert:
return sqlparser.String(stmt.Table), nil
case *sqlparser.Select:
return sqlparser.String(stmt.From), nil
return sqlparser.ToString(stmt.From), nil
}

return "", fmt.Errorf("%w: %+v", ErrUnsupportedQuery, sqlparser.String(stmt))
Expand Down
11 changes: 9 additions & 2 deletions go/vt/vtgate/planbuilder/joinGen4.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ limitations under the License.
package planbuilder

import (
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
"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/semantics"
)
Expand Down Expand Up @@ -91,12 +93,17 @@ func (j *joinGen4) Primitive() engine.Primitive {

// Inputs implements the logicalPlan interface
func (j *joinGen4) Inputs() []logicalPlan {
panic("implement me")
return []logicalPlan{j.Left, j.Right}
}

// Rewrite implements the logicalPlan interface
func (j *joinGen4) Rewrite(inputs ...logicalPlan) error {
panic("implement me")
if len(inputs) != 2 {
return vterrors.New(vtrpcpb.Code_INTERNAL, "wrong number of children")
}
j.Left = inputs[0]
j.Right = inputs[1]
return nil
}

// ContainsTables implements the logicalPlan interface
Expand Down
32 changes: 26 additions & 6 deletions go/vt/vtgate/planbuilder/route_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,13 +261,10 @@ func planHorizon(sel *sqlparser.Select, plan logicalPlan, semTable *semantics.Se
return plan, nil
}

// TODO real horizon planning to be done
if sel.Distinct {
return nil, semantics.Gen4NotSupportedF("DISTINCT")
}
if sel.GroupBy != nil {
return nil, semantics.Gen4NotSupportedF("GROUP BY")
if err := checkUnsupportedConstructs(sel); err != nil {
return nil, err
}

qp, err := createQPFromSelect(sel)
if err != nil {
return nil, err
Expand All @@ -278,6 +275,16 @@ func planHorizon(sel *sqlparser.Select, plan logicalPlan, semTable *semantics.Se
}
}

for _, expr := range qp.aggrExprs {
funcExpr, ok := expr.Expr.(*sqlparser.FuncExpr)
if !ok {
return nil, vterrors.New(vtrpcpb.Code_INTERNAL, "expected an aggregation here")
}
if funcExpr.Distinct {
return nil, semantics.Gen4NotSupportedF("distinct aggregation")
}
}

if len(qp.aggrExprs) > 0 {
plan, err = planAggregations(qp, plan, semTable)
if err != nil {
Expand Down Expand Up @@ -308,6 +315,19 @@ func createSingleShardRoutePlan(sel *sqlparser.Select, rb *route) {
}
}

func checkUnsupportedConstructs(sel *sqlparser.Select) error {
if sel.Distinct {
return semantics.Gen4NotSupportedF("DISTINCT")
}
if sel.GroupBy != nil {
return semantics.Gen4NotSupportedF("GROUP BY")
}
if sel.Having != nil {
return semantics.Gen4NotSupportedF("HAVING")
}
return nil
}

func pushJoinPredicate(exprs []sqlparser.Expr, tree joinTree, semTable *semantics.SemTable) (joinTree, error) {
switch node := tree.(type) {
case *routePlan:
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/symtab.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func (st *symtab) AddTable(t *table) error {
st.singleRoute = nil
}
if _, ok := st.tables[t.alias]; ok {
return fmt.Errorf("duplicate symbol: %s", sqlparser.String(t.alias))
return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.NonUniqTable, "Not unique table/alias: '%s'", t.alias.Name.String())
}
st.tables[t.alias] = t
st.tableNames = append(st.tableNames, t.alias)
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/planbuilder/testdata/aggr_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1342,3 +1342,4 @@ Gen4 plan same as above
# syntax error detected by planbuilder
"select count(distinct *) from user"
"syntax error: count(distinct *)"
Gen4 plan same as above
Loading