analyzer: fix crash with aliases used multiple times

The reorder projection rule recreates the projection with the table
columns and aliases. When an alias is used more than once in a
node expression, for example, a filter (a=1 or a=2) it is added twice.
This caused the second time to add nil as the aliases where deleted
after adding them to projections list.

Now the columns are added only once.

Signed-off-by: Javi Fontan <[email protected]>
(cherry picked from commit 1dddaa4)

Author: jfontan

sql/analyzer/optimization_rules.go

@@ -120,8 +120,10 @@ func reorderProjection(ctx *sql.Context, a *Analyzer, n sql.Node) (sql.Node, err
 			}
 
 			for _, col := range requiredColumns {
-				projections = append(projections, newColumns[col])
-				delete(newColumns, col)
+				if c, ok := newColumns[col]; ok {
+					projections = append(projections, c)
+					delete(newColumns, col)
+				}
 			}
 
 			child = plan.NewProject(projections, child)

sql/analyzer/optimization_rules_test.go

@@ -11,68 +11,126 @@ import (
 )
 
 func TestReorderProjection(t *testing.T) {
-	require := require.New(t)
 	f := getRule("reorder_projection")
 
 	table := mem.NewTable("mytable", sql.Schema{{
 		Name: "i", Source: "mytable", Type: sql.Int64,
 	}})
 
-	node := plan.NewProject(
-		[]sql.Expression{
-			expression.NewGetFieldWithTable(0, sql.Int64, "mytable", "i", false),
-			expression.NewAlias(expression.NewLiteral(1, sql.Int64), "foo"),
-			expression.NewAlias(expression.NewLiteral(2, sql.Int64), "bar"),
-		},
-		plan.NewSort(
-			[]plan.SortField{
-				{Column: expression.NewUnresolvedColumn("foo")},
-			},
-			plan.NewFilter(
-				expression.NewEquals(
-					expression.NewLiteral(1, sql.Int64),
-					expression.NewUnresolvedColumn("bar"),
+	testCases := []struct {
+		name     string
+		project  sql.Node
+		expected sql.Node
+	}{
+		{
+			"sort",
+			plan.NewProject(
+				[]sql.Expression{
+					expression.NewGetFieldWithTable(0, sql.Int64, "mytable", "i", false),
+					expression.NewAlias(expression.NewLiteral(1, sql.Int64), "foo"),
+					expression.NewAlias(expression.NewLiteral(2, sql.Int64), "bar"),
+				},
+				plan.NewSort(
+					[]plan.SortField{
+						{Column: expression.NewUnresolvedColumn("foo")},
+					},
+					plan.NewFilter(
+						expression.NewEquals(
+							expression.NewLiteral(1, sql.Int64),
+							expression.NewUnresolvedColumn("bar"),
+						),
+						plan.NewResolvedTable(table),
+					),
 				),
-				plan.NewResolvedTable(table),
 			),
-		),
-	)
-
-	expected := plan.NewProject(
-		[]sql.Expression{
-			expression.NewGetFieldWithTable(0, sql.Int64, "mytable", "i", false),
-			expression.NewGetField(2, sql.Int64, "foo", false),
-			expression.NewGetField(1, sql.Int64, "bar", false),
-		},
-		plan.NewSort(
-			[]plan.SortField{{Column: expression.NewGetField(2, sql.Int64, "foo", false)}},
 			plan.NewProject(
 				[]sql.Expression{
 					expression.NewGetFieldWithTable(0, sql.Int64, "mytable", "i", false),
+					expression.NewGetField(2, sql.Int64, "foo", false),
 					expression.NewGetField(1, sql.Int64, "bar", false),
+				},
+				plan.NewSort(
+					[]plan.SortField{{Column: expression.NewGetField(2, sql.Int64, "foo", false)}},
+					plan.NewProject(
+						[]sql.Expression{
+							expression.NewGetFieldWithTable(0, sql.Int64, "mytable", "i", false),
+							expression.NewGetField(1, sql.Int64, "bar", false),
+							expression.NewAlias(expression.NewLiteral(1, sql.Int64), "foo"),
+						},
+						plan.NewFilter(
+							expression.NewEquals(
+								expression.NewLiteral(1, sql.Int64),
+								expression.NewGetField(1, sql.Int64, "bar", false),
+							),
+							plan.NewProject(
+								[]sql.Expression{
+									expression.NewGetFieldWithTable(0, sql.Int64, "mytable", "i", false),
+									expression.NewAlias(expression.NewLiteral(2, sql.Int64), "bar"),
+								},
+								plan.NewResolvedTable(table),
+							),
+						),
+					),
+				),
+			),
+		},
+		{
+			"use alias twice",
+			plan.NewProject(
+				[]sql.Expression{
 					expression.NewAlias(expression.NewLiteral(1, sql.Int64), "foo"),
 				},
 				plan.NewFilter(
-					expression.NewEquals(
-						expression.NewLiteral(1, sql.Int64),
-						expression.NewGetField(1, sql.Int64, "bar", false),
+					expression.NewOr(
+						expression.NewEquals(
+							expression.NewLiteral(1, sql.Int64),
+							expression.NewUnresolvedColumn("foo"),
+						),
+						expression.NewEquals(
+							expression.NewLiteral(1, sql.Int64),
+							expression.NewUnresolvedColumn("foo"),
+						),
+					),
+					plan.NewResolvedTable(table),
+				),
+			),
+			plan.NewProject(
+				[]sql.Expression{
+					expression.NewGetField(1, sql.Int64, "foo", false),
+				},
+				plan.NewFilter(
+					expression.NewOr(
+						expression.NewEquals(
+							expression.NewLiteral(1, sql.Int64),
+							expression.NewGetField(1, sql.Int64, "foo", false),
+						),
+						expression.NewEquals(
+							expression.NewLiteral(1, sql.Int64),
+							expression.NewGetField(1, sql.Int64, "foo", false),
+						),
 					),
 					plan.NewProject(
 						[]sql.Expression{
 							expression.NewGetFieldWithTable(0, sql.Int64, "mytable", "i", false),
-							expression.NewAlias(expression.NewLiteral(2, sql.Int64), "bar"),
+							expression.NewAlias(expression.NewLiteral(1, sql.Int64), "foo"),
 						},
 						plan.NewResolvedTable(table),
 					),
 				),
 			),
-		),
-	)
+		},
+	}
 
-	result, err := f.Apply(sql.NewEmptyContext(), NewDefault(nil), node)
-	require.NoError(err)
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			require := require.New(t)
 
-	require.Equal(expected, result)
+			result, err := f.Apply(sql.NewEmptyContext(), NewDefault(nil), tt.project)
+			require.NoError(err)
+
+			require.Equal(tt.expected, result)
+		})
+	}
 }
 
 func TestEraseProjection(t *testing.T) {