generalize the tuple detection for projectsions and groupbys

Signed-off-by: Juanjo Alvarez <[email protected]>

Author: Juanjo Alvarez

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,41 @@ 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 {
+				fmt.Println("XXX grouping: ", c)
+				_, ok := c.(expression.Tuple)
+				if ok {
+					return node, ErrTupleProjected.New()
+				}
+			}
+			for _, c := range groupby.Aggregate {
+				fmt.Println("XXX aggregate: ", c)
+				_, 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{} {

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) {

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 ()?")
 )