Merge pull request #556 from dolthub/aaron/fix-indexed-subquery-expre…

…ssion-in-lifted-join-condition sql/plan/indexed_table_access.go: Change static index lookup nodes to not keep returning their unused key expressions.
dolthub · Sep 21, 2021 · 52ff812 · 52ff812
2 parents e83ad3b + b81cb90
commit 52ff812
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 11 deletions.
diff --git a/enginetest/queries.go b/enginetest/queries.go
@@ -388,6 +388,12 @@ var QueryTests = []QueryTest{
 			{3, 3.0},
 		},
 	},
+	{
+		Query: `SELECT * FROM mytable mt JOIN othertable ot ON ot.i2 = (SELECT i2 FROM othertable WHERE s2 = "second") AND mt.i = ot.i2 JOIN mytable mt2 ON mt.i = mt2.i`,
+		Expected: []sql.Row{
+			{2, "second row", "second", 2, 2, "second row"},
+		},
+	},
 	{
 		Query: `SELECT a.column_0, b.column_1 FROM (values row(1+1,2+2), row(floor(1.5),concat("a","b"))) a
 			join (values row(2,4), row(1.0,"ab")) b on a.column_0 = b.column_0 and a.column_0 = b.column_0

diff --git a/enginetest/query_plans.go b/enginetest/query_plans.go
@@ -1327,9 +1327,10 @@ var PlanTests = []QueryPlanTest{
 		AND (SELECT i2 FROM othertable where i2 = i) IS NOT NULL`,
 		ExpectedPlan: "Project(mt.i)\n" +
 			" └─ Filter((NOT((Project(mytable.i)\n" +
-			"     └─ Filter((mytable.i = mt.i) AND (mytable.i > 2))\n" +
+			"     └─ Filter(mytable.i = mt.i)\n" +
 			"         └─ Projected table access on [i]\n" +
-			"             └─ IndexedTableAccess(mytable on [mytable.i])\n" +
+			"             └─ Filter(mytable.i > 2)\n" +
+			"                 └─ IndexedTableAccess(mytable on [mytable.i])\n" +
 			"    ) IS NULL)) AND (NOT((Project(othertable.i2)\n" +
 			"     └─ Filter(othertable.i2 = mt.i)\n" +
 			"         └─ Projected table access on [i2]\n" +
@@ -1362,8 +1363,8 @@ var PlanTests = []QueryPlanTest{
 		ExpectedPlan: "Sort(t1.pk ASC, t2.pk2 ASC)\n" +
 			" └─ Project(t1.pk, t2.pk2, (Limit(1)\n" +
 			"     └─ Project(one_pk.pk)\n" +
-			"         └─ Filter(one_pk.pk = 1)\n" +
-			"             └─ Projected table access on [pk]\n" +
+			"         └─ Projected table access on [pk]\n" +
+			"             └─ Filter(one_pk.pk = 1)\n" +
 			"                 └─ IndexedTableAccess(one_pk on [one_pk.pk])\n" +
 			"    ) as (SELECT pk from one_pk where pk = 1 limit 1))\n" +
 			"     └─ CrossJoin\n" +

diff --git a/sql/analyzer/pushdown_test.go b/sql/analyzer/pushdown_test.go
@@ -760,7 +760,7 @@ func TestPushdownIndex(t *testing.T) {
 							plan.NewResolvedTable(table2, nil, nil),
 							mustIndexLookup(idxTable2I2.Get(21)),
 							idxTable2I2,
-							[]sql.Expression{gfCol(0, mytable2I)},
+							[]sql.Expression{gfCol(3, mytable2I)},
 						),
 					),
 				),
@@ -917,7 +917,7 @@ func TestPushdownIndex(t *testing.T) {
 							plan.NewResolvedTable(table2, nil, nil),
 							mustIndexLookup(idxTable2I2.Get(21)),
 							idxTable2I2,
-							[]sql.Expression{gfCol(0, mytable2I)},
+							[]sql.Expression{gfCol(3, mytable2I)},
 						),
 					),
 				),
@@ -1063,7 +1063,7 @@ func TestPushdownIndex(t *testing.T) {
 								plan.NewResolvedTable(table2, nil, nil),
 								mustIndexLookup(idxTable2I2.Get(21)),
 								idxTable2I2,
-								[]sql.Expression{gfColAlias(0, mytable2I, "t2")},
+								[]sql.Expression{gfColAlias(3, mytable2I, "t2")},
 							),
 						),
 					),
@@ -1150,7 +1150,7 @@ func TestPushdownIndex(t *testing.T) {
 								plan.NewResolvedTable(table2, nil, nil),
 								mustIndexLookup(idxTable2I2.Get(21)),
 								idxTable2I2,
-								[]sql.Expression{gfColAlias(0, mytable2I, "t2")},
+								[]sql.Expression{gfColAlias(3, mytable2I, "t2")},
 							),
 						),
 					),
@@ -1455,7 +1455,7 @@ func TestPushdownIndex(t *testing.T) {
 									plan.NewResolvedTable(table2, nil, nil),
 									mustIndexLookup(idxTable2I2.Get(21)),
 									idxTable2I2,
-									[]sql.Expression{gfColAlias(0, mytable2I, "t2")},
+									[]sql.Expression{gfColAlias(3, mytable2I, "t2")},
 								),
 							),
 						),

diff --git a/sql/plan/indexed_table_access.go b/sql/plan/indexed_table_access.go
@@ -117,24 +117,36 @@ func formatIndexDecoratorString(idx sql.Index) string {
 }
 
 func (i *IndexedTableAccess) DebugString() string {
+	if i.lookup != nil {
+		return fmt.Sprintf("IndexedTableAccess(%s on %s, using fields %s)", i.Name(), formatIndexDecoratorString(i.index), "STATIC LOOKUP("+sql.DebugString(i.lookup)+")")
+	}
 	keyExprs := make([]string, len(i.keyExprs))
 	for j := range i.keyExprs {
 		keyExprs[j] = sql.DebugString(i.keyExprs[j])
 	}
-	return fmt.Sprintf("IndexedTableAccess(%s, using fields %s)", i.Name(), strings.Join(keyExprs, ", "))
+	return fmt.Sprintf("IndexedTableAccess(%s on %s, using fields %s)", i.Name(), formatIndexDecoratorString(i.index), strings.Join(keyExprs, ", "))
 }
 
 // Expressions implements sql.Expressioner
 func (i *IndexedTableAccess) Expressions() []sql.Expression {
+	if i.lookup != nil {
+		return nil
+	}
 	return i.keyExprs
 }
 
 // WithExpressions implements sql.Expressioner
 func (i *IndexedTableAccess) WithExpressions(exprs ...sql.Expression) (sql.Node, error) {
+	if i.lookup != nil {
+		if len(exprs) != 0 {
+			return nil, sql.ErrInvalidChildrenNumber.New(i, 0, len(i.keyExprs))
+		}
+		n := *i
+		return &n, nil
+	}
 	if len(exprs) != len(i.keyExprs) {
 		return nil, sql.ErrInvalidChildrenNumber.New(i, len(exprs), len(i.keyExprs))
 	}
-
 	return &IndexedTableAccess{
 		ResolvedTable: i.ResolvedTable,
 		index:         i.index,