From 1ba012b8d6fb67459d7a73542daf80735313cd92 Mon Sep 17 00:00:00 2001 From: Juanjo Alvarez Date: Wed, 17 Apr 2019 12:26:51 +0200 Subject: [PATCH] generalize the tuple detection for projectsions and groupbys Signed-off-by: Juanjo Alvarez --- sql/analyzer/resolve_columns.go | 48 +++++++++++++++++----------- sql/analyzer/resolve_columns_test.go | 48 +++++++++++++++++++++++++--- sql/analyzer/rules.go | 4 +-- 3 files changed, 74 insertions(+), 26 deletions(-) diff --git a/sql/analyzer/resolve_columns.go b/sql/analyzer/resolve_columns.go index 80d24f883..4c16fa5d8 100644 --- a/sql/analyzer/resolve_columns.go +++ b/sql/analyzer/resolve_columns.go @@ -5,7 +5,7 @@ import ( "sort" "strings" - errors "gopkg.in/src-d/go-errors.v1" + "gopkg.in/src-d/go-errors.v1" "gopkg.in/src-d/go-mysql-server.v0/sql" "gopkg.in/src-d/go-mysql-server.v0/sql/expression" "gopkg.in/src-d/go-mysql-server.v0/sql/plan" @@ -38,29 +38,39 @@ func checkAliases(ctx *sql.Context, a *Analyzer, n sql.Node) (sql.Node, error) { return n, err } -func checkDistinctNoTuples(ctx *sql.Context, a *Analyzer, node sql.Node) (sql.Node, error) { - span, _ := ctx.Span("no_distinct_tuples") +func checkNoTuplesProjected(ctx *sql.Context, a *Analyzer, node sql.Node) (sql.Node, error) { + span, _ := ctx.Span("no_tuples_projected") defer span.Finish() - a.Log("check no tuples as distinct projection, node of type: %T", node) - var err error - if node, ok := node.(*plan.Distinct); ok { - _, err = node.TransformUp(func(node sql.Node) (sql.Node, error) { - project, ok := node.(*plan.Project) - if ok { - for _, col := range project.Projections { - _, ok := col.(expression.Tuple) - if ok { - return node, ErrDistinctTuple.New() - } + a.Log("check no tuples as in projection, node of type: %T", node) + return node.TransformUp(func(node sql.Node) (sql.Node, error) { + project, ok := node.(*plan.Project) + if ok { + for _, col := range project.Projections { + _, ok := col.(expression.Tuple) + if ok { + return node, ErrTupleProjected.New() } } + } + groupby, ok := node.(*plan.GroupBy) + if ok { + for _, c := range groupby.Grouping { + _, ok := c.(expression.Tuple) + if ok { + return node, ErrTupleProjected.New() + } + } + for _, c := range groupby.Aggregate { + _, ok := c.(expression.Tuple) + if ok { + return node, ErrTupleProjected.New() + } + } + } - return node, nil - }) - } - - return node, err + return node, nil + }) } func lookForAliasDeclarations(node sql.Expressioner) map[string]struct{} { diff --git a/sql/analyzer/resolve_columns_test.go b/sql/analyzer/resolve_columns_test.go index a7245ec1f..bf3d5814a 100644 --- a/sql/analyzer/resolve_columns_test.go +++ b/sql/analyzer/resolve_columns_test.go @@ -75,9 +75,9 @@ func TestMisusedAlias(t *testing.T) { require.EqualError(err, ErrMisusedAlias.New("alias_i").Error()) } -func TestDistinctNoTuples(t *testing.T) { +func TestNoTuplesProjected(t *testing.T) { require := require.New(t) - f := getRule("check_distinct_no_tuples") + f := getRule("no_tuples_projected") table := mem.NewTable("mytable", sql.Schema{ {Name: "i", Type: sql.Int32}, @@ -89,10 +89,48 @@ func TestDistinctNoTuples(t *testing.T) { expression.NewLiteral(2, sql.Int64), ), }, plan.NewResolvedTable(table)) - d := plan.NewDistinct(node) - _, err := f.Apply(sql.NewEmptyContext(), nil, d) - require.EqualError(err, ErrDistinctTuple.New().Error()) + _, err := f.Apply(sql.NewEmptyContext(), nil, node) + require.EqualError(err, ErrTupleProjected.New().Error()) +} + +func TestNoTuplesGroupBy(t *testing.T) { + require := require.New(t) + f := getRule("no_tuples_projected") + + table := mem.NewTable("mytable", sql.Schema{ + {Name: "i", Type: sql.Int32}, + }) + + node := plan.NewGroupBy([]sql.Expression{ + expression.NewUnresolvedColumn("a"), + expression.NewUnresolvedColumn("b"), + }, + []sql.Expression{ + expression.NewTuple( + expression.NewLiteral(1, sql.Int64), + expression.NewLiteral(2, sql.Int64), + ), + }, + plan.NewResolvedTable(table)) + + _, err := f.Apply(sql.NewEmptyContext(), nil, node) + require.EqualError(err, ErrTupleProjected.New().Error()) + + node = plan.NewGroupBy([]sql.Expression{ + expression.NewTuple( + expression.NewLiteral(1, sql.Int64), + expression.NewLiteral(2, sql.Int64), + ), + }, + []sql.Expression{ + expression.NewUnresolvedColumn("a"), + expression.NewUnresolvedColumn("b"), + }, + plan.NewResolvedTable(table)) + + _, err = f.Apply(sql.NewEmptyContext(), nil, node) + require.EqualError(err, ErrTupleProjected.New().Error()) } func TestQualifyColumns(t *testing.T) { diff --git a/sql/analyzer/rules.go b/sql/analyzer/rules.go index a9ddda1fd..2c0099f7c 100644 --- a/sql/analyzer/rules.go +++ b/sql/analyzer/rules.go @@ -28,7 +28,7 @@ var OnceBeforeDefault = []Rule{ {"resolve_subqueries", resolveSubqueries}, {"resolve_tables", resolveTables}, {"check_aliases", checkAliases}, - {"check_distinct_no_tuples", checkDistinctNoTuples}, + {"no_tuples_projected", checkNoTuplesProjected}, } // OnceAfterDefault contains the rules to be applied just once after the @@ -67,5 +67,5 @@ var ( // ErrMisusedAlias is returned when a alias is defined and used in the same projection. ErrMisusedAlias = errors.NewKind("column %q does not exist in scope, but there is an alias defined in" + " this projection with that name. Aliases cannot be used in the same projection they're defined in") - ErrDistinctTuple = errors.NewKind("tuple used as DISTINCT argument, remove the ()") + ErrTupleProjected = errors.NewKind("unexpected tuple found, maybe remove the ()?") )