plan: propagate constant over outer join #7794

eurekaka · 2018-09-27T08:48:59Z

What problem does this PR solve?

Fix #7098, inner join scenario is handled by #7276, this PR is for outer join.

What is changed and how it works?

Purpose is to generate more conditions we can push down to children plan nodes by constant propagation.

Below is a good example to illustrate this purpose.
Before this PR:

create table t1(id int primary key, a int, b int);
create table t2(id int primary key, a int, b int);

mysql> explain select * from t1 left join t2 on t1.a = t2.a where t1.a = 1;
+-------------------------+----------+------+-------------------------------------------------------------------------+
| id                      | count    | task | operator info                                                           |
+-------------------------+----------+------+-------------------------------------------------------------------------+
| HashLeftJoin_7          | 12.50    | root | left outer join, inner:TableReader_12, equal:[eq(test.t1.a, test.t2.a)] |
| ├─TableReader_10        | 10.00    | root | data:Selection_9                                                        |
| │ └─Selection_9         | 10.00    | cop  | eq(test.t1.a, 1)                                                        |
| │   └─TableScan_8       | 10000.00 | cop  | table:t1, range:[-inf,+inf], keep order:false, stats:pseudo             |
| └─TableReader_12        | 10000.00 | root | data:TableScan_11                                                       |
|   └─TableScan_11        | 10000.00 | cop  | table:t2, range:[-inf,+inf], keep order:false, stats:pseudo             |
+-------------------------+----------+------+-------------------------------------------------------------------------+
6 rows in set (0.00 sec)

mysql> explain select * from t1 left join t2 on t1.a = t2.a where t1.a > 1;
+-------------------------+----------+------+-------------------------------------------------------------------------+
| id                      | count    | task | operator info                                                           |
+-------------------------+----------+------+-------------------------------------------------------------------------+
| HashLeftJoin_7          | 4166.67  | root | left outer join, inner:TableReader_12, equal:[eq(test.t1.a, test.t2.a)] |
| ├─TableReader_10        | 3333.33  | root | data:Selection_9                                                        |
| │ └─Selection_9         | 3333.33  | cop  | gt(test.t1.a, 1)                                                        |
| │   └─TableScan_8       | 10000.00 | cop  | table:t1, range:[-inf,+inf], keep order:false, stats:pseudo             |
| └─TableReader_12        | 10000.00 | root | data:TableScan_11                                                       |
|   └─TableScan_11        | 10000.00 | cop  | table:t2, range:[-inf,+inf], keep order:false, stats:pseudo             |
+-------------------------+----------+------+-------------------------------------------------------------------------+
6 rows in set (0.00 sec)

after this PR:

mysql> explain select * from t1 left join t2 on t1.a = t2.a where t1.a = 1;
+-------------------------+----------+------+-------------------------------------------------------------+
| id                      | count    | task | operator info                                               |
+-------------------------+----------+------+-------------------------------------------------------------+
| HashLeftJoin_7          | 100.00   | root | left outer join, inner:TableReader_13                       |
| ├─TableReader_10        | 10.00    | root | data:Selection_9                                            |
| │ └─Selection_9         | 10.00    | cop  | eq(test.t1.a, 1)                                            |
| │   └─TableScan_8       | 10000.00 | cop  | table:t1, range:[-inf,+inf], keep order:false, stats:pseudo |
| └─TableReader_13        | 10.00    | root | data:Selection_12                                           |
|   └─Selection_12        | 10.00    | cop  | eq(1, test.t2.a)                                            |
|     └─TableScan_11      | 10000.00 | cop  | table:t2, range:[-inf,+inf], keep order:false, stats:pseudo |
+-------------------------+----------+------+-------------------------------------------------------------+
7 rows in set (0.00 sec)

mysql> explain select * from t1 left join t2 on t1.a = t2.a where t1.a > 1;
+-------------------------+----------+------+-------------------------------------------------------------------------+
| id                      | count    | task | operator info                                                           |
+-------------------------+----------+------+-------------------------------------------------------------------------+
| HashLeftJoin_7          | 4166.67  | root | left outer join, inner:TableReader_13, equal:[eq(test.t1.a, test.t2.a)] |
| ├─TableReader_10        | 3333.33  | root | data:Selection_9                                                        |
| │ └─Selection_9         | 3333.33  | cop  | gt(test.t1.a, 1)                                                        |
| │   └─TableScan_8       | 10000.00 | cop  | table:t1, range:[-inf,+inf], keep order:false, stats:pseudo             |
| └─TableReader_13        | 3333.33  | root | data:Selection_12                                                       |
|   └─Selection_12        | 3333.33  | cop  | gt(test.t2.a, 1)                                                        |
|     └─TableScan_11      | 10000.00 | cop  | table:t2, range:[-inf,+inf], keep order:false, stats:pseudo             |
+-------------------------+----------+------+-------------------------------------------------------------------------+
7 rows in set (0.00 sec)

More filters are pushed down to scan nodes, hence less rows in join operator.

Steps to propagate constant over outer join:

extract outerCol = const from join conditions and filter conditions,
substitute outerCol in join conditions with const;
extract outerCol = innerCol from join conditions, derive new join
conditions based on this column equal condition and outerCol related
expressions in join conditions and filter conditions;

As we have enhanced outer join simplification in #7696, this PR should be a complete solution to cover all cases in the current constant propagation framework IMHO.

Check List

Tests

Unit test
Integration test

Code changes

Has exported function/method change

Side effects

Increased code complexity

- extract `outerCol = const` from join conditions and filter conditions, substitute `outerCol` in join conditions with `const`; - extract `outerCol = innerCol` from join conditions, derive new join conditions based on this column equal condition and `outerCol` related expressions in join conditions and filter conditions;

do not propagate filter with aux column

eurekaka · 2018-09-27T15:59:45Z

/run-all-tests

eurekaka · 2018-09-28T01:46:05Z

/run-sqllogic-test

eurekaka · 2018-09-28T17:30:22Z

/run-all-tests

eurekaka · 2018-09-29T08:32:32Z

/run-all-tests

eurekaka · 2018-09-29T09:12:00Z

@zz-jason @winoros @XuHuaiyu PTAL

alivxxx · 2018-09-30T07:56:11Z

planner/core/join_explain_test.go

+	testleak.AfterTest(c)()
+}
+
+func (s *testSuite) TestOuterJoinPropConst(c *C) {


Can we put it to explain test?

I prefer putting them into explain test as well, but I learned that explain test does not contribute to code coverage test...

But it doesn't contribue to coverage test as well, since most of the changes is at expression, but this test is for planner/core. It is better to add unit test for the PropConstOverOuterJoin.

Make sense, patch updated.

alivxxx · 2018-09-30T08:07:31Z

expression/constant_propagation.go

+				value, err := EvalBool(s.ctx, []Expression{con}, chunk.Row{})
+				terror.Log(errors.Trace(err))
+				if !value {
+					if fConds {


Seems like it can be simplified to s.setConds2ConstFalse(true, fConds), and the first parameter is always true.

Yes, I put 2 parameters here just because it looks more clear logically IMHO. If we just pass in a false fConds, then in the function we set the jConds to constant false, it may be a little bit confusing.

eurekaka · 2018-10-09T13:03:28Z

/run-all-tests

eurekaka · 2018-10-10T01:38:29Z

/run-integration-ddl-test

eurekaka · 2018-10-10T01:49:11Z

conflicts resolved, PTAL @zz-jason @winoros @XuHuaiyu

winoros · 2018-10-10T07:27:26Z

expression/constant_propagation.go

+}
+
+func (s *basePropConstSolver) getColID(col *Column) int {
+	code := col.HashCode(nil)


HashCode is actually hash the UniqueID to byte slice. You can use it directly.

Make sense, will do it.

winoros · 2018-10-10T07:56:55Z

expression/constant_propagation.go

+}
+
+// deriveConds given `outerCol = innerCol`, derive new expression for specified conditions.
+func (s *propOuterJoinConstSolver) deriveConds(outerCol, innerCol *Column, schema *Schema, fCondsOffset int, visited []bool, fConds bool) []bool {


visited = s.deriveConds(outerCol, innerCol, mergedSchema, lenJoinConds, visited, false)

Here lenJoinConds is passed. Why here named fCondsOffset?

Hmm, because it is indeed an offset inside deriveConds definition, while it is computed from lenJoinConds in caller...

zz-jason · 2018-10-11T03:58:18Z

expression/constant_propagation.go

+type propOuterJoinConstSolver struct {
+	basePropConstSolver
+	jConds      []Expression
+	fConds      []Expression


how about:

s/jConds/joinConds/

s/fConds/filterConds/

zz-jason · 2018-10-11T12:27:27Z

expression/constant_propagation.go

+func (s *propOuterJoinConstSolver) solve(joinConds, filterConds []Expression) ([]Expression, []Expression) {
+	cols := make([]*Column, 0, len(joinConds)+len(filterConds))
+	for _, cond := range joinConds {
+		s.jConds = append(s.jConds, SplitCNFItems(cond)...)


maybe this can be removed?

If we remove this SplitCNFItems, we have to make sure all the join conditions have been split before we come to here? For most cases this assumption should hold, because planBuilder::buildJoin would split the on clause, but there may exist exceptions, e.g, in handleCompareSubquery -> buildSemiApply, I am not sure whether the condition has been split already. Also, this assumption may break for newly added code in future, so I prefer to keep this SplitCNFItems.

zz-jason · 2018-10-11T12:29:17Z

expression/constant_propagation.go

+		s.insertCol(col)
+	}
+	if len(s.columns) > MaxPropagateColsCnt {
+		log.Warnf("[const_propagation] Too many columns: column count is %d, max count is %d.", len(s.columns), MaxPropagateColsCnt)


s/const_propagation/const_propagation_over_outerjoin/?

eurekaka · 2018-10-12T02:41:51Z

@zz-jason comments addressed, PTAL

alivxxx · 2018-10-15T07:31:27Z

expression/constant_propagation.go

+	return nil, nil
+}
+
+// validColEqualCond checks if expression is column equql condition that we can use for constant


equql -> equal.

alivxxx

LGTM

zz-jason · 2018-10-15T11:03:22Z

planner/core/rule_predicate_push_down.go

+
+// outerJoinPropConst propagates constant equal and column equal conditions over outer join.
+func (p *LogicalJoin) outerJoinPropConst(predicates []expression.Expression) []expression.Expression {
+	if p.JoinType == InnerJoin || p.JoinType == SemiJoin || p.JoinType == AntiSemiJoin {


this check can be removed?

zz-jason · 2018-10-15T11:20:14Z

expression/constant_propagation.go

+}
+
+// pickNewEQCondsFunc picks constant equal expression from specified conditions.
+func (s *propOuterJoinConstSolver) pickNewEQCondsFunc(retMapper map[int]*Constant, visited []bool, filterConds bool) map[int]*Constant {


how about s/pickNewEQCondsFunc/pickEQCondsOnOuterCol/?

zz-jason · 2018-10-15T11:23:44Z

expression/constant_propagation.go

+		if col == nil {
+			if con, ok = cond.(*Constant); ok {
+				value, err := EvalBool(s.ctx, []Expression{con}, chunk.Row{})
+				terror.Log(errors.Trace(err))


if err is not nil, should we break the process of propagation?

zz-jason · 2018-10-15T11:26:35Z

expression/constant_propagation.go

+				value, err := EvalBool(s.ctx, []Expression{con}, chunk.Row{})
+				terror.Log(errors.Trace(err))
+				if !value {
+					if filterConds {


the following if statement can be simplified to:

s.setConds2ConstFalse(true, filterConds)

zz-jason · 2018-10-15T11:26:50Z

expression/constant_propagation.go

+		visited[i+condsOffset] = true
+		updated, foreverFalse := s.tryToUpdateEQList(col, con)
+		if foreverFalse {
+			if filterConds {


zz-jason · 2018-10-15T11:28:45Z

expression/constant_propagation.go

+	innerSchema *Schema
+}
+
+func (s *propOuterJoinConstSolver) setConds2ConstFalse(joinConds, filterConds bool) {


maybe the first parameter can be removed, because it's always true:

5 expression/constant_propagation.go|367 col 9| s.setConds2ConstFalse(true, true) 6 expression/constant_propagation.go|369 col 9| s.setConds2ConstFalse(true, false) 7 expression/constant_propagation.go|384 col 7| s.setConds2ConstFalse(true, true) 8 expression/constant_propagation.go|386 col 7| s.setConds2ConstFalse(true, false)

zz-jason

LGTM

eurekaka · 2018-10-16T05:57:06Z

/run-all-tests

bugfix fixed pingcap#7518 expression: MySQL compatible current_user function (pingcap#7801) plan: propagate constant over outer join (pingcap#7794) - extract `outerCol = const` from join conditions and filter conditions, substitute `outerCol` in join conditions with `const`; - extract `outerCol = innerCol` from join conditions, derive new join conditions based on this column equal condition and `outerCol` related expressions in join conditions and filter conditions; util/timeutil: fix data race caused by forgetting set stats lease to 0 (pingcap#7901) stats: handle ddl event for partition table (pingcap#7903) plan: implement Operand and Pattern of cascades planner. (pingcap#7910) planner: not convert to TableDual if empty range is derived from deferred constants (pingcap#7808) plan: move projEliminate behind aggEliminate (pingcap#7909) admin: fix admin check table bug of byte compare (pingcap#7887) * admin: remove reflect deepEqual stats: fix panic caused by empty histogram (pingcap#7912) plan: fix panic caused by empty schema of LogicalTableDual (pingcap#7906) * fix drop view if exist error (pingcap#7833) executor: refine `explain analyze` (pingcap#7888) executor: add an variable to compatible with MySQL insert for OGG (pingcap#7863) expression: maintain `DeferredExpr` in aggressive constant folding. (pingcap#7915) stats: fix histogram boundaries overflow error (pingcap#7883) ddl:support the definition of `null` change to `not null` using `alter table` (pingcap#7771) * ddl:support the definition of null change to not null using alter table ddl: add check when create table with foreign key. (pingcap#7885) * ddl: add check when create table with foreign key planner: eliminate if null on non null column (pingcap#7924) executor: fix a bug in point get (pingcap#7934) planner, executor: refine ColumnPrune for LogicalUnionAll (pingcap#7930) executor: fix panic when limit is too large (pingcap#7936) ddl: add TiDB version to metrics (pingcap#7902) stats: limit the length of sample values (pingcap#7931) vendor: update tipb (pingcap#7893) planner: support the Group and GroupExpr for the cascades planner (pingcap#7917) store/tikv: log more information when other err occurs (pingcap#7948) types: fix date time parse (pingcap#7933) ddl: just print error message when ddl job is normal to calcel, to eliminate noisy log (pingcap#7875) stats: update delta info for partition table (pingcap#7947) explaintest: add explain test for partition pruning (pingcap#7505) util: move disjoint set to util package (pingcap#7950) util: add PreAlloc4Row and Insert for Chunk and List (pingcap#7916) executor: add the slow log for commit (pingcap#7951) expression: add builtin json_keys (pingcap#7776) privilege: add USAGE in `show grants` for mysql compatibility (pingcap#7955) ddl: fix invailid ddl job panic (pingcap#7940) *: move ast.NewValueExpr to standalone parser_driver package (pingcap#7952) Make the ast package get rid of the dependency of types.Datum server: allow cors http request (pingcap#7939) *: move `Statement` and `RecordSet` from ast to sqlexec package (pingcap#7970) pr suggestion update executor/aggfuncs: split unit tests to corresponding file (pingcap#7993) store/tikv: fix typo (pingcap#7990) executor, planner: clone proj schema for different children in buildProj4Union (pingcap#7999) executor: let information_schema be the first database in ShowDatabases (pingcap#7938) stats: use local feedback for partition table (pingcap#7963) executor: add unit test for aggfuncs (pingcap#7966) server: add log for binary execute statement (pingcap#7987) admin: refine admin check decoder (pingcap#7862) executor: improve wide table insert & update performance (pingcap#7935) ddl: fix reassigned partition id in `truncate table` does not take effect (pingcap#7919) fix reassigned partition id in truncate table does not take effect add changelog for 2.1.0 rc4 (pingcap#8020) *: make parser package dependency as small as possible (pingcap#7989) parser: support `:=` in the `set` syntax (pingcap#8018) According to MySQL document, `set` use the = assignment operator, but the := assignment operator is also permitted stats: garbage collect stats for partition table (pingcap#7962) docs: add the proposal for the column pool (pingcap#7988) expression: refine built-in func truncate to support uint arg (pingcap#8000) stats: support show stats for partition table (pingcap#8023) stats: update error rate for partition table (pingcap#8022) stats: fix estimation for out of range point queries (pingcap#8015) *: move parser to a separate repository (pingcap#8036) executor: fix wrong result when index join on union scan. (pingcap#8031) Do not modify Plan of dataReaderBuilder directly, because it would impact next batch of outer rows, as well as other concurrent inner workers. Instead, build a local child builder to store the child plan. planner: fix a panic of a cached prepared statement with IndexScan (pingcap#8017) *: fix the issue of executing DDL after executing SQL failure in txn (pingcap#8044) * ddl, executor: fix the issue of executing DDL after executing SQL failure in txn add unit test remove debug info add like evaluator case sensitive test ddl, domain: make schema correct after canceling jobs (pingcap#7997) unit test fix code format proposal: maintaining histograms in plan. (pingcap#7605) support _tidb_rowid for table scan range (pingcap#8047) var rename fix

eurekaka added 7 commits September 13, 2018 17:57

update unit test result due to code change

1e56122

add comments for exported function to shutup gofmt

f18e344

add unit test

552f717

fix left outer semi join bug

c951009

do not propagate filter with aux column

refactor function and add comments

a7e133c

move unit test to expression package

383b2ab

eurekaka added type/enhancement The issue or PR belongs to an enhancement. status/WIP sig/planner SIG: Planner labels Sep 27, 2018

eurekaka removed the status/WIP label Sep 27, 2018

eurekaka added the status/all tests passed label Sep 29, 2018

alivxxx reviewed Sep 30, 2018

View reviewed changes

eurekaka force-pushed the const_prop_outer_join branch 2 times, most recently from 668c5bc to 383b2ab Compare October 9, 2018 13:00

winoros reviewed Oct 10, 2018

View reviewed changes

eurekaka added 4 commits October 10, 2018 16:22

Merge branch 'master' into const_prop_outer_join

1cb5043

use UniqueID to substitute hash code of Column

4c58055

Merge branch 'master' into const_prop_outer_join

ba4583f

Merge branch 'master' into const_prop_outer_join

d481b8c

zz-jason reviewed Oct 11, 2018

View reviewed changes

Merge branch 'master' into const_prop_outer_join

e886557

rename variables

4a123a0

address comments

00f27b7

alivxxx reviewed Oct 15, 2018

View reviewed changes

eurekaka added 2 commits October 15, 2018 15:35

Merge branch 'master' into const_prop_outer_join

2f03d21

fix typo

f4e55bb

alivxxx reviewed Oct 15, 2018

View reviewed changes

alivxxx added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 15, 2018

zz-jason reviewed Oct 15, 2018

View reviewed changes

eurekaka added 2 commits October 16, 2018 13:33

Merge branch 'master' into const_prop_outer_join

e5f0691

address comments

6c01c82

zz-jason approved these changes Oct 16, 2018

View reviewed changes

eurekaka added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 16, 2018

Merge branch 'master' into const_prop_outer_join

f69b6f8

eurekaka merged commit 6064d64 into pingcap:master Oct 16, 2018

eurekaka deleted the const_prop_outer_join branch October 16, 2018 06:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plan: propagate constant over outer join #7794

plan: propagate constant over outer join #7794

eurekaka commented Sep 27, 2018 •

edited

Loading

eurekaka commented Sep 27, 2018

eurekaka commented Sep 28, 2018

eurekaka commented Sep 28, 2018

eurekaka commented Sep 29, 2018

eurekaka commented Sep 29, 2018

alivxxx Sep 30, 2018

eurekaka Sep 30, 2018

alivxxx Sep 30, 2018

eurekaka Sep 30, 2018

alivxxx Sep 30, 2018

eurekaka Sep 30, 2018

eurekaka commented Oct 9, 2018

eurekaka commented Oct 10, 2018

eurekaka commented Oct 10, 2018

winoros Oct 10, 2018

eurekaka Oct 10, 2018

winoros Oct 10, 2018

eurekaka Oct 10, 2018

zz-jason Oct 11, 2018

zz-jason Oct 11, 2018

eurekaka Oct 12, 2018

zz-jason Oct 12, 2018

zz-jason Oct 11, 2018

eurekaka commented Oct 12, 2018

alivxxx Oct 15, 2018

alivxxx left a comment

zz-jason Oct 15, 2018

zz-jason Oct 15, 2018

zz-jason Oct 15, 2018

zz-jason Oct 15, 2018

zz-jason Oct 15, 2018

zz-jason Oct 15, 2018

zz-jason left a comment

eurekaka commented Oct 16, 2018

plan: propagate constant over outer join #7794

plan: propagate constant over outer join #7794

Conversation

eurekaka commented Sep 27, 2018 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

eurekaka commented Sep 27, 2018

eurekaka commented Sep 28, 2018

eurekaka commented Sep 28, 2018

eurekaka commented Sep 29, 2018

eurekaka commented Sep 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eurekaka commented Oct 9, 2018

eurekaka commented Oct 10, 2018

eurekaka commented Oct 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eurekaka commented Oct 12, 2018

Choose a reason for hiding this comment

alivxxx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zz-jason left a comment

Choose a reason for hiding this comment

eurekaka commented Oct 16, 2018

eurekaka commented Sep 27, 2018 •

edited

Loading