Merge pull request #597 from dolthub/aaron/analyzer-avoid-filter-push…

…down-over-window sql/analyzer: pushdown_filters: Avoid pushing down filters below Window nodes.
dolthub · Oct 25, 2021 · be5218f · be5218f
2 parents 7778444 + 3b6affd
commit be5218f
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 0 deletions.
diff --git a/enginetest/queries.go b/enginetest/queries.go
@@ -598,6 +598,39 @@ var QueryTests = []QueryTest{
 			{3, 1},
 		},
 	},
+	{
+		// In this case, the parser and analyzer collaborate to place the filter below the WINDOW function,
+		// and the window sees the filtered rows.
+		Query: "SELECT ROW_NUMBER() OVER (ORDER BY s2 ASC) idx, i2, s2 FROM othertable WHERE s2 <> 'second' ORDER BY i2 ASC",
+		Expected: []sql.Row{
+			{2, 1, "third"},
+			{1, 3, "first"},
+		},
+	},
+	{
+		// In this case, the analyzer should not push the filter below the window function.
+		Query: "SELECT * FROM (SELECT ROW_NUMBER() OVER (ORDER BY s2 ASC) idx, i2, s2 FROM othertable ORDER BY i2 ASC) a WHERE s2 <> 'second'",
+		Expected: []sql.Row{
+			{3, 1, "third"},
+			{1, 3, "first"},
+		},
+	},
+	{
+		// Same as above, but with an available index access on i2
+		Query: "SELECT ROW_NUMBER() OVER (ORDER BY s2 ASC) idx, i2, s2 FROM othertable WHERE i2 < 2 OR i2 > 2 ORDER BY i2 ASC",
+		Expected: []sql.Row{
+			{2, 1, "third"},
+			{1, 3, "first"},
+		},
+	},
+	{
+		// Same as above, but with an available index access on i2
+		Query: "SELECT * FROM (SELECT ROW_NUMBER() OVER (ORDER BY s2 ASC) idx, i2, s2 FROM othertable ORDER BY i2 ASC) a WHERE i2 < 2 OR i2 > 2",
+		Expected: []sql.Row{
+			{3, 1, "third"},
+			{1, 3, "first"},
+		},
+	},
 	{
 		Query: `WITH mt1 as (select i,s FROM mytable)
 			SELECT mtouter.i, (select s from mt1 where s = mtouter.s) FROM mt1 as mtouter where mtouter.i > 1 order by 1`,

diff --git a/enginetest/query_plans.go b/enginetest/query_plans.go
@@ -1431,6 +1431,49 @@ var PlanTests = []QueryPlanTest{
 			"                 └─ Table(two_pk)\n" +
 			"",
 	},
+	{
+		Query: `SELECT ROW_NUMBER() OVER (ORDER BY s2 ASC) idx, i2, s2 FROM othertable WHERE s2 <> 'second' ORDER BY i2 ASC`,
+		ExpectedPlan: "Sort(othertable.i2 ASC)\n" +
+			" └─ Project(row_number() over ( order by [othertable.s2, idx=0, type=TEXT, nullable=false] ASC) as idx, othertable.i2, othertable.s2)\n" +
+			"     └─ Window(row_number() over ( order by [othertable.s2, idx=0, type=TEXT, nullable=false] ASC), othertable.i2, othertable.s2)\n" +
+			"         └─ Filter(NOT((othertable.s2 = \"second\")))\n" +
+			"             └─ Projected table access on [i2 s2]\n" +
+			"                 └─ IndexedTableAccess(othertable on [othertable.s2])\n" +
+			"",
+	},
+	{
+		Query: `SELECT * FROM (SELECT ROW_NUMBER() OVER (ORDER BY s2 ASC) idx, i2, s2 FROM othertable ORDER BY i2 ASC) a WHERE s2 <> 'second'`,
+		ExpectedPlan: "SubqueryAlias(a)\n" +
+			" └─ Filter(NOT((othertable.s2 = \"second\")))\n" +
+			"     └─ Sort(othertable.i2 ASC)\n" +
+			"         └─ Project(row_number() over ( order by [othertable.s2, idx=0, type=TEXT, nullable=false] ASC) as idx, othertable.i2, othertable.s2)\n" +
+			"             └─ Window(row_number() over ( order by [othertable.s2, idx=0, type=TEXT, nullable=false] ASC), othertable.i2, othertable.s2)\n" +
+			"                 └─ Projected table access on [s2 i2]\n" +
+			"                     └─ Table(othertable)\n" +
+			"",
+	},
+	{
+		// In theory it is fine to use the index here, but we currently do not.
+		Query: `SELECT ROW_NUMBER() OVER (ORDER BY s2 ASC) idx, i2, s2 FROM othertable WHERE i2 < 2 OR i2 > 2 ORDER BY i2 ASC`,
+		ExpectedPlan: "Sort(othertable.i2 ASC)\n" +
+			" └─ Project(row_number() over ( order by [othertable.s2, idx=0, type=TEXT, nullable=false] ASC) as idx, othertable.i2, othertable.s2)\n" +
+			"     └─ Window(row_number() over ( order by [othertable.s2, idx=0, type=TEXT, nullable=false] ASC), othertable.i2, othertable.s2)\n" +
+			"         └─ Filter((othertable.i2 < 2) OR (othertable.i2 > 2))\n" +
+			"             └─ Projected table access on [i2 s2]\n" +
+			"                 └─ IndexedTableAccess(othertable on [othertable.i2])\n" +
+			"",
+	},
+	{
+		Query: `SELECT * FROM (SELECT ROW_NUMBER() OVER (ORDER BY s2 ASC) idx, i2, s2 FROM othertable ORDER BY i2 ASC) a WHERE i2 < 2 OR i2 > 2`,
+		ExpectedPlan: "SubqueryAlias(a)\n" +
+			" └─ Filter((othertable.i2 < 2) OR (othertable.i2 > 2))\n" +
+			"     └─ Sort(othertable.i2 ASC)\n" +
+			"         └─ Project(row_number() over ( order by [othertable.s2, idx=0, type=TEXT, nullable=false] ASC) as idx, othertable.i2, othertable.s2)\n" +
+			"             └─ Window(row_number() over ( order by [othertable.s2, idx=0, type=TEXT, nullable=false] ASC), othertable.i2, othertable.s2)\n" +
+			"                 └─ Projected table access on [i2 s2]\n" +
+			"                     └─ Table(othertable)\n" +
+			"",
+	},
 	{
 		Query: `DELETE FROM two_pk WHERE c1 > 1`,
 		ExpectedPlan: "Delete\n" +

diff --git a/sql/analyzer/pushdown.go b/sql/analyzer/pushdown.go
@@ -34,6 +34,10 @@ func pushdownFilters(ctx *sql.Context, a *Analyzer, n sql.Node, scope *Scope) (s
 		return n, nil
 	}
 
+	return pushdownFiltersAtNode(ctx, a, n, scope)
+}
+
+func pushdownFiltersAtNode(ctx *sql.Context, a *Analyzer, n sql.Node, scope *Scope) (sql.Node, error) {
 	indexes, err := getIndexesByTable(ctx, a, n, scope)
 	if err != nil {
 		return nil, err
@@ -168,6 +172,11 @@ func filterPushdownChildSelector(c plan.TransformContext) bool {
 	switch n := c.Parent.(type) {
 	case *plan.TableAlias:
 		return false
+	case *plan.Window:
+		// Windows operate across the rows they see and cannot have
+		// filters pushed below them. Instead, the step will be run
+		// again by the Transform function, starting at this node.
+		return false
 	case *plan.IndexedJoin:
 		if n.JoinType() == plan.JoinTypeLeft || n.JoinType() == plan.JoinTypeRight {
 			return c.ChildNum == 0
@@ -264,6 +273,14 @@ func transformPushdownFilters(ctx *sql.Context, a *Analyzer, n sql.Node, scope *
 
 			// Then move filter predicates directly above their respective tables in joins
 			return pushdownAboveTables(node, filters)
+		case *plan.Window:
+			// Analyze below the Window in isolation to push down
+			// any relevant indexes, for example.
+			child, err := pushdownFiltersAtNode(ctx, a, n.Child, scope)
+			if err != nil {
+				return nil, err
+			}
+			return n.WithChildren(child)
 		default:
 			return n, nil
 		}
@@ -333,6 +350,12 @@ func convertFiltersToIndexedAccess(
 			// TableAlias, but not to the resolved table directly
 			// beneath it.
 			return false
+		case *plan.Window:
+			// Windows operate across the rows they see and cannot
+			// have filters pushed below them. If there is an index
+			// pushdown, it will get picked up in the isolated pass
+			// run by the filters pushdown transform.
+			return false
 		}
 		return true
 	}