Skip to content

Commit

Permalink
sql: store physical props in indexJoinNode
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Justin Jaffray committed Jun 6, 2018
1 parent 7c8d6f6 commit c0d6c65
Show file tree
Hide file tree
Showing 10 changed files with 113 additions and 13 deletions.
4 changes: 4 additions & 0 deletions pkg/sql/expand_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/index_join.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/select_index
Original file line number Diff line number Diff line change
Expand Up @@ -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))
----
Expand Down Expand Up @@ -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
28 changes: 19 additions & 9 deletions pkg/sql/opt/exec/execbuilder/relational_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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),
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/exec/execbuilder/testdata/distsql_indexjoin
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/select_index
Original file line number Diff line number Diff line change
Expand Up @@ -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
----
Expand Down Expand Up @@ -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 · ·
2 changes: 1 addition & 1 deletion pkg/sql/opt/exec/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/opt_exec_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -402,6 +402,9 @@ func (ef *execFactory) ConstructLookupJoin(
primaryKeyPrefix: primaryKeyPrefix,
colIDtoRowIndex: colIDtoRowIndex,
},
props: physicalProps{
ordering: reqOrder,
},
}, nil
}

Expand Down
13 changes: 13 additions & 0 deletions pkg/sql/opt_needed.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,26 @@ 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
// using the table scanNode.
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
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/plan_physical_props.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit c0d6c65

Please sign in to comment.