Repository: src-d/go-mysql-server
Pull request #672 (2019-04-11 17:14:39)

Fix validation rule to detect tuples in projections or groupbys
Fixes src-d/gitbase/issues/785

Modifies a rule to check if distinct have a tuple argument (`DISTINCT(1,2)`), recursively. Unrelatedly, it also fix ordering in the README for the FROM_BASE64 function (my fault).

There are also a couple unrelated files with `go fmt` changes which I decided to add to this PR since it won't hurt.

Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>

erizocosmico: It should not only be distinct, but anything that is projected. A tuple simply can't be projected (neither in Project nor GroupBy)
juanjux: @erizocosmico change to detect tuples in projections and groupbys implemented, PTAL when you can.
juanjux: Update: there was already a validation rule checking that there are not tuples in Projections or GroupBys (https://github.com/src-d/go-mysql-server/blob/master/sql/analyzer/validation_rules.go#L198), it even have tests with tuples (https://github.com/src-d/go-mysql-server/blob/master/sql/analyzer/validation_rules_test.go#L234) but adding a test with a DISTINCT->Projection->Tuple doesn't detect the error (and it crash like the issue reported if you do it on the REPL), so I'm not investigating why this existing rule doesn't work for this case.

Patch:
diff --git a/README.md b/README.md
index 805a07e..d26fa0c 100644
--- a/README.md
+++ b/README.md
@@ -76,6 +76,7 @@ We support and actively test against certain third-party clients to ensure compa
 |`DAYOFWEEK(date)`|Returns the day of the week of the given date.|
 |`DAYOFYEAR(date)`|Returns the day of the year of the given date.|
 |`FLOOR(number)`|Return the largest integer value that is less than or equal to `number`.|
+|`FROM_BASE64(str)`|Decodes the base64-encoded string str.|
 |`HOUR(date)`|Returns the hours of the given date.|
 |`IFNULL(expr1, expr2)`|If expr1 is not NULL, IFNULL() returns expr1; otherwise it returns expr2.|
 |`IS_BINARY(blob)`|Returns whether a BLOB is a binary file or not.|
@@ -110,7 +111,6 @@ We support and actively test against certain third-party clients to ensure compa
 |`SUBSTRING(str, pos, [len])`|Return a substring from the provided string starting at `pos` with a length of `len` characters. If no `len` is provided, all characters from `pos` until the end will be taken.|
 |`SUM(expr)`|Returns the sum of expr in all rows.|
 |`TO_BASE64(str)`|Encodes the string str in base64 format.|
-|`FROM_BASE64(str)`|Decodes the base64-encoded string str.|
 |`TRIM(str)`|Returns the string str with all spaces removed.|
 |`UPPER(str)`|Returns the string str with all characters in upper case.|
 |`WEEKDAY(date)`|Returns the weekday of the given date.|
diff --git a/sql/analyzer/optimization_rules.go b/sql/analyzer/optimization_rules.go
index f879534..e45c141 100644
--- a/sql/analyzer/optimization_rules.go
+++ b/sql/analyzer/optimization_rules.go
@@ -1,7 +1,7 @@
 package analyzer
 
 import (
-	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"
diff --git a/sql/analyzer/resolve_columns.go b/sql/analyzer/resolve_columns.go
index 2e6cb43..2758726 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"
diff --git a/sql/analyzer/validation_rules.go b/sql/analyzer/validation_rules.go
index c1cbadc..288163f 100644
--- a/sql/analyzer/validation_rules.go
+++ b/sql/analyzer/validation_rules.go
@@ -3,7 +3,7 @@ package analyzer
 import (
 	"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/expression/function"
@@ -195,27 +195,36 @@ func validateSchema(t *plan.ResolvedTable) error {
 	return nil
 }
 
-func validateProjectTuples(ctx *sql.Context, a *Analyzer, n sql.Node) (sql.Node, error) {
-	span, _ := ctx.Span("validate_project_tuples")
-	defer span.Finish()
+func findProjectTuples(n sql.Node) (sql.Node, error) {
+	if n == nil {
+		return n, nil
+	}
 
 	switch n := n.(type) {
-	case *plan.Project:
-		for i, e := range n.Projections {
+	case *plan.Project, *plan.GroupBy:
+		for i, e := range n.(sql.Expressioner).Expressions() {
 			if sql.IsTuple(e.Type()) {
 				return nil, ErrProjectTuple.New(i+1, sql.NumColumns(e.Type()))
 			}
 		}
-	case *plan.GroupBy:
-		for i, e := range n.Aggregate {
-			if sql.IsTuple(e.Type()) {
-				return nil, ErrProjectTuple.New(i+1, sql.NumColumns(e.Type()))
+	default:
+		for _, ch := range n.Children() {
+			_, err := findProjectTuples(ch)
+			if err != nil {
+				return nil, err
 			}
 		}
 	}
+
 	return n, nil
 }
 
+func validateProjectTuples(ctx *sql.Context, a *Analyzer, n sql.Node) (sql.Node, error) {
+	span, _ := ctx.Span("validate_project_tuples")
+	defer span.Finish()
+	return findProjectTuples(n)
+}
+
 func validateCaseResultTypes(ctx *sql.Context, a *Analyzer, n sql.Node) (sql.Node, error) {
 	span, ctx := ctx.Span("validate_case_result_types")
 	defer span.Finish()


Test patch:
diff --git a/sql/analyzer/validation_rules_test.go b/sql/analyzer/validation_rules_test.go
index 85993b8..b1fa359 100644
--- a/sql/analyzer/validation_rules_test.go
+++ b/sql/analyzer/validation_rules_test.go
@@ -235,11 +235,38 @@ func TestValidateProjectTuples(t *testing.T) {
 			plan.NewProject([]sql.Expression{
 				expression.NewTuple(
 					expression.NewLiteral(1, sql.Int64),
-					expression.NewLiteral(1, sql.Int64),
+					expression.NewLiteral(2, sql.Int64),
 				),
 			}, nil),
 			false,
 		},
+		{
+			"distinct with a 2 elem tuple inside the project",
+			plan.NewDistinct(
+				plan.NewProject([]sql.Expression{
+					expression.NewTuple(
+						expression.NewLiteral(1, sql.Int64),
+						expression.NewLiteral(2, sql.Int64),
+					),
+				}, nil)),
+			false,
+		},
+		{
+			"alias with a tuple",
+			plan.NewProject(
+				[]sql.Expression{
+					expression.NewAlias(
+						expression.NewTuple(
+							expression.NewLiteral(1, sql.Int64),
+							expression.NewLiteral(2, sql.Int64),
+						),
+						"foo",
+					),
+				},
+				plan.NewUnresolvedTable("dual", ""),
+			),
+			false,
+		},
 		{
 			"groupby with no tuple",
 			plan.NewGroupBy([]sql.Expression{