From c0d6c65131cd76f9a7fa773a17cf527bc95a9777 Mon Sep 17 00:00:00 2001 From: Justin Jaffray Date: Mon, 4 Jun 2018 15:15:59 -0400 Subject: [PATCH] sql: store physical props in indexJoinNode The change in #25292 was incomplete, this change finishes that work to store the physical props on the indexJoinNode and not simply pass through to the index scan. Release note: None --- pkg/sql/expand_plan.go | 4 +++ pkg/sql/index_join.go | 9 +++++ .../testdata/logic_test/select_index | 34 +++++++++++++++++++ .../exec/execbuilder/relational_builder.go | 28 ++++++++++----- .../execbuilder/testdata/distsql_indexjoin | 2 +- .../exec/execbuilder/testdata/select_index | 27 +++++++++++++++ pkg/sql/opt/exec/factory.go | 2 +- pkg/sql/opt_exec_factory.go | 5 ++- pkg/sql/opt_needed.go | 13 +++++++ pkg/sql/plan_physical_props.go | 2 +- 10 files changed, 113 insertions(+), 13 deletions(-) diff --git a/pkg/sql/expand_plan.go b/pkg/sql/expand_plan.go index a003a9dfb6b4..dd8f70623ab4 100644 --- a/pkg/sql/expand_plan.go +++ b/pkg/sql/expand_plan.go @@ -702,7 +702,11 @@ func (p *planner) simplifyOrderings(plan planNode, usefulOrdering sqlbase.Column } case *indexJoinNode: + // Passing through usefulOrdering here is fine because indexJoinNodes + // produced by the heuristic planner always have the same schema as the + // underlying table. n.index.props.trim(usefulOrdering) + n.props.trim(usefulOrdering) n.table.props = physicalProps{} case *unionNode: diff --git a/pkg/sql/index_join.go b/pkg/sql/index_join.go index d28d73d0186b..987d42071856 100644 --- a/pkg/sql/index_join.go +++ b/pkg/sql/index_join.go @@ -95,6 +95,10 @@ type indexJoinNode struct { // There is a 1-1 correspondence between cols and resultColumns. resultColumns sqlbase.ResultColumns + // props contains the physical properties provided by this node. + // These are pre-computed at construction time. + props physicalProps + run indexJoinRun } @@ -210,6 +214,11 @@ func (p *planner) makeIndexJoin( primaryKeyPrefix: primaryKeyPrefix, colIDtoRowIndex: colIDtoRowIndex, }, + // We need to store the physical props in this node because they might be + // different from the underlying index scan in plans produced by the + // optimizer (though they will be the same in plans produced by the + // heuristic planner). + props: planPhysicalProps(indexScan), } return node, indexScan, nil diff --git a/pkg/sql/logictest/testdata/logic_test/select_index b/pkg/sql/logictest/testdata/logic_test/select_index index dc388362c687..c77bb77036d5 100644 --- a/pkg/sql/logictest/testdata/logic_test/select_index +++ b/pkg/sql/logictest/testdata/logic_test/select_index @@ -293,6 +293,18 @@ SELECT * FROM xy WHERE x IN (NULL, 1, 2) 1 NULL 1 1 +statement ok +CREATE TABLE ef (e INT, f INT, INDEX(f)) + +statement ok +INSERT INTO ef VALUES (NULL, 1), (1, 1) + +query I rowsort +SELECT e FROM ef WHERE f > 0 AND f < 2 ORDER BY f +---- +NULL +1 + query II SELECT * FROM xy WHERE (x, y) IN ((NULL, NULL), (1, NULL), (NULL, 1), (1, 1), (1, 2)) ---- @@ -483,3 +495,25 @@ SELECT a FROM t2 WHERE b = 2 OR ((b BETWEEN 2 AND 1) AND ((s != 'a') OR (s = 'a' 4 5 6 + +statement ok +CREATE TABLE t3 (k INT PRIMARY KEY, v INT, w INT, INDEX v(v)) + +statement ok +INSERT INTO t3 VALUES + (10, 50, 1), + (30, 40, 2), + (50, 30, 3), + (70, 20, 4), + (90, 10, 5), + (110, 0, 6), + (130, -10, 7) + +query I +SELECT w FROM t3 WHERE v > 0 AND v < 100 ORDER BY v +---- +5 +4 +3 +2 +1 diff --git a/pkg/sql/opt/exec/execbuilder/relational_builder.go b/pkg/sql/opt/exec/execbuilder/relational_builder.go index 0050832b045e..0b8223cafb7c 100644 --- a/pkg/sql/opt/exec/execbuilder/relational_builder.go +++ b/pkg/sql/opt/exec/execbuilder/relational_builder.go @@ -215,14 +215,7 @@ func (*Builder) getColumns( return needed, output } -func (b *Builder) buildScan(ev memo.ExprView) (execPlan, error) { - def := ev.Private().(*memo.ScanOpDef) - md := ev.Metadata() - tab := md.Table(def.Table) - - needed, output := b.getColumns(md, def.Cols, def.Table) - res := execPlan{outputCols: output} - +func (b *Builder) makeSQLOrdering(res execPlan, ev memo.ExprView) sqlbase.ColumnOrdering { var reqOrder sqlbase.ColumnOrdering if reqProps := ev.Physical(); len(reqProps.Ordering) > 0 { reqOrder = make(sqlbase.ColumnOrdering, len(reqProps.Ordering)) @@ -236,6 +229,19 @@ func (b *Builder) buildScan(ev memo.ExprView) (execPlan, error) { } } + return reqOrder +} + +func (b *Builder) buildScan(ev memo.ExprView) (execPlan, error) { + def := ev.Private().(*memo.ScanOpDef) + md := ev.Metadata() + tab := md.Table(def.Table) + + needed, output := b.getColumns(md, def.Cols, def.Table) + res := execPlan{outputCols: output} + + reqOrder := b.makeSQLOrdering(res, ev) + root, err := b.factory.ConstructScan( tab, tab.Index(def.Index), @@ -556,7 +562,11 @@ func (b *Builder) buildLookupJoin(ev memo.ExprView) (execPlan, error) { needed, output := b.getColumns(md, def.Cols, def.Table) res := execPlan{outputCols: output} - node, err := b.factory.ConstructLookupJoin(input.root, md.Table(def.Table), needed) + reqOrder := b.makeSQLOrdering(res, ev) + + needed.UnionWith(ev.Physical().Ordering.ColSet()) + + node, err := b.factory.ConstructLookupJoin(input.root, md.Table(def.Table), needed, reqOrder) if err != nil { return execPlan{}, err } diff --git a/pkg/sql/opt/exec/execbuilder/testdata/distsql_indexjoin b/pkg/sql/opt/exec/execbuilder/testdata/distsql_indexjoin index 7212ed49a959..95963ec43f7e 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/distsql_indexjoin +++ b/pkg/sql/opt/exec/execbuilder/testdata/distsql_indexjoin @@ -36,7 +36,7 @@ https://cockroachdb.github.io/distsqlplan/decode.html?eJyslM1uozAURvfzFKNvO3cUMO query T SELECT "URL" FROM [EXPLAIN (DISTSQL) SELECT w FROM t WHERE v > 10 AND v < 50 ORDER BY v] ---- -https://cockroachdb.github.io/distsqlplan/decode.html?eJyslL9uqzAYR_f7FFe_9foq2JA_ZWJNh6SKulUMFH-KkBKMbFO1qnj3KjhSTdU4RGHE9vmOz4A_UStJm-JIBukLOBgEGGIwJGCYI2dotCrJGKVPRxywlu9II4aqblrrlm1lD4QUSkvSJMEgyRbVoZ-b8X_Iu5yhVJqQfp_eqP-qma0Gp_OOQbX2PDlnMLbYE9K4Y56de_ZfBj8XrwfaUSFJz6LhZd4yC4Zta9O_GWeZwCUhv0X4qKr67EuGvkZXx0J_eNb4olIMlGJ8I5-k8YrQa5xP1RiPbxSTNF4Reo2LqRqT8Y3xJI1XhF7jcqrGKKzckWlUbWjUnx6dngqSe3JPi1GtLulJq7LXuM9tz_ULkox1uw_uY127rdMFfZgHYRGGRRCOBjD_CcdBOAmbk3vM8yC8CJsX95iXQXgVNq9uMufdn68AAAD__5mfNlw= +https://cockroachdb.github.io/distsqlplan/decode.html?eJyslL9uqzAYR_f7FFe_ta6CDflTJtZ0SKqoW8VA8acIKcHINlWrinevgiPVVK1DFEZsn-_4DPgTtZK0KY5kkL6Ag0GAIQZDAoY5coZGq5KMUfp0xAFr-Y40YqjqprVu2Vb2QEihtCRNEgySbFEd-rmZuEPe5Qyl0oT0-_RG3atmthqczjsG1drz5JzB2GJPSOOOeXbu2X8Z_Fy8HmhHhSQ9i4aXecssGLatTf9nnGUCfwn5NcJHVdVnXzL0Nbo6FvrDs8YhqxhYxfhMPknmBaGXOZ8wMx6fKSbJvCD0MhcTZibjM-NJMi8IvczlhJlR2Loj06ja0Ki_Pjo9GyT35J4Zo1pd0pNWZa9xn9ue6xckGet2H9zHunZbpwv6MA_CIgyLIBwNYP4TjoNwEjYnt5jnQXgRNi9uMS-D8CpsXl1lzrt_XwEAAP__NMo41Q== # The single join reader should be on node 5, and doesn't need to output v. query T diff --git a/pkg/sql/opt/exec/execbuilder/testdata/select_index b/pkg/sql/opt/exec/execbuilder/testdata/select_index index 1aba4d80aef8..a29e07eb100a 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/select_index +++ b/pkg/sql/opt/exec/execbuilder/testdata/select_index @@ -834,6 +834,18 @@ index-join · · (x, y) · └── scan · · (x, y) · · table xy@primary · · +query TTTTT +EXPLAIN (VERBOSE) SELECT x FROM xy WHERE y > 0 AND y < 2 ORDER BY y +---- +render · · (x) · + │ render 0 x · · + └── index-join · · (x, y, rowid[hidden]) +y + ├── scan · · (y, rowid[hidden]) +y + │ table xy@xy_y_idx · · + │ spans /1-/2 · · + └── scan · · (x, y, rowid[hidden]) · +· table xy@primary · · + query TTTTT EXPLAIN (VERBOSE) SELECT * FROM xy WHERE y IS DISTINCT FROM NULL ---- @@ -1217,3 +1229,18 @@ render · · (a) · │ spans /2-/3 · · └── scan · · (a, b, s) · · table t2@primary · · + +statement ok +CREATE TABLE t3 (k INT PRIMARY KEY, v INT, w INT, INDEX v(v)) + +query TTTTT +EXPLAIN (VERBOSE) SELECT w FROM t3 WHERE v > 0 AND v < 100 ORDER BY v +---- +render · · (w) · + │ render 0 w · · + └── index-join · · (v, w) +v + ├── scan · · (k, v) +v + │ table t3@v · · + │ spans /1-/100 · · + └── scan · · (v, w) · +· table t3@primary · · diff --git a/pkg/sql/opt/exec/factory.go b/pkg/sql/opt/exec/factory.go index b0d777fde621..4b5673c0d354 100644 --- a/pkg/sql/opt/exec/factory.go +++ b/pkg/sql/opt/exec/factory.go @@ -107,7 +107,7 @@ type Factory interface { ConstructOrdinality(input Node, colName string) (Node, error) // ConstructLookupJoin returns a node that performs a lookup join. - ConstructLookupJoin(input Node, table opt.Table, cols ColumnOrdinalSet) (Node, error) + ConstructLookupJoin(input Node, table opt.Table, cols ColumnOrdinalSet, reqOrder sqlbase.ColumnOrdering) (Node, error) // ConstructLimit returns a node that implements LIMIT and/or OFFSET on the // results of the given node. If one or the other is not needed, then it is diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go index bae6b2c26a62..15d33f93599c 100644 --- a/pkg/sql/opt_exec_factory.go +++ b/pkg/sql/opt_exec_factory.go @@ -351,7 +351,7 @@ func (ef *execFactory) ConstructOrdinality(input exec.Node, colName string) (exe // ConstructLookupJoin is part of the exec.Factory interface. func (ef *execFactory) ConstructLookupJoin( - input exec.Node, table opt.Table, cols exec.ColumnOrdinalSet, + input exec.Node, table opt.Table, cols exec.ColumnOrdinalSet, reqOrder sqlbase.ColumnOrdering, ) (exec.Node, error) { tabDesc := table.(*optTable).desc colCfg := scanColumnsConfig{ @@ -402,6 +402,9 @@ func (ef *execFactory) ConstructLookupJoin( primaryKeyPrefix: primaryKeyPrefix, colIDtoRowIndex: colIDtoRowIndex, }, + props: physicalProps{ + ordering: reqOrder, + }, }, nil } diff --git a/pkg/sql/opt_needed.go b/pkg/sql/opt_needed.go index 1d541bb326a3..7174e1814beb 100644 --- a/pkg/sql/opt_needed.go +++ b/pkg/sql/opt_needed.go @@ -60,6 +60,11 @@ func setNeededColumns(plan planNode, needed []bool) { // Currently all the needed result columns are provided by the // table sub-source; from the index sub-source we only need the PK // columns sufficient to configure the table sub-source. + + // The columns in the underlying indexes will always line up with the + // columns in the indexJoinNode, since this plan was produced by the + // heuristic planner. + // TODO(radu/knz): see the comments at the start of index_join.go, // perhaps this can be optimized to utilize the column values // already provided by the index instead of re-retrieving them @@ -67,6 +72,14 @@ func setNeededColumns(plan planNode, needed []bool) { setNeededColumns(n.table, needed) setNeededColumns(n.index, n.primaryKeyColumns) + for i, colNeeded := range needed { + if colNeeded { + n.resultColumns[i].Omitted = false + } else { + n.resultColumns[i].Omitted = true + } + } + case *unionNode: if !n.emitAll { // For UNION (as opposed to UNION ALL) we have to check for diff --git a/pkg/sql/plan_physical_props.go b/pkg/sql/plan_physical_props.go index d2ecc60965a5..6afbc111a937 100644 --- a/pkg/sql/plan_physical_props.go +++ b/pkg/sql/plan_physical_props.go @@ -36,7 +36,7 @@ func planPhysicalProps(plan planNode) physicalProps { case *spoolNode: return planPhysicalProps(n.source) case *indexJoinNode: - return planPhysicalProps(n.index) + return n.props case *serializeNode: return planPhysicalProps(n.source) case *deleteNode: