plan: add hint to force to choose hash join. #5315
Conversation
parser/parser.y
Outdated
| @@ -421,6 +421,7 @@ import ( | |||
| statsHistograms "STATS_HISTOGRAMS" | |||
| statsBuckets "STATS_BUCKETS" | |||
| tidb "TIDB" | |||
| tidbHASHJOIN "TIDB_HASHJOIN" | |||
There was a problem hiding this comment.
As convention, we should use TIDB_HJ
|
/run-all-tests |
|
/run-all-tests |
plan/join_reorder.go
Outdated
| @@ -29,7 +30,7 @@ func tryToGetJoinGroup(j *LogicalJoin) ([]LogicalPlan, bool) { | |||
| // 2. not inner join | |||
| // 3. forced merge join | |||
| // 4. forced index nested loop join | |||
| if j.reordered || !j.cartesianJoin || j.preferMergeJoin || j.preferINLJ > 0 { | |||
| if j.reordered || !j.cartesianJoin || bits.OnesCount(j.preferJoinType) > 0 { | |||
There was a problem hiding this comment.
Should we update the comment?
executor/join_test.go
Outdated
| @@ -147,6 +147,8 @@ func (s *testSuite) TestJoin(c *C) { | |||
|
|
|||
| // Test that two conflict hints will return error. | |||
| _, err = tk.Exec("select /*+ TIDB_INLJ(t) TIDB_SMJ(t) */ * from t join t1 on t.a=t1.a") | |||
| _, err = tk.Exec("select /*+ TIDB_INLJ(t) TIDB_HASHJOIN(t) */ from t join t1 on t.a=t1.a") | |||
| _, err = tk.Exec("select /*+ TIDB_SMJ(t) TIDB_HASHJOIN(t) */ from t join t1 on t.a=t1.a") | |||
There was a problem hiding this comment.
The err will be overwritten, so check just once is not enough.
plan/logical_plan_builder.go
Outdated
| } | ||
| // If there're multiple join hints, they're conflict. | ||
| if bits.OnesCount(joinPlan.preferJoinType) > 1 { | ||
| b.err = errors.New("Optimizer Hints is conflict") |
There was a problem hiding this comment.
make error message more specific ? like: Optimizer Hints conflict, at most one of {TIDB_INLJ, TIDB_SMJ, TIDB_HASHJOIN} join hints should be specified.
|
/run-all-tests |
|
/run-all-tests |
plan/planbuilder.go
Outdated
| // Which it joins on with depend on sequence of traverse | ||
| // and without reorder, user might adjust themselves. | ||
| // This is similar to MySQL hints. | ||
| for _, tableName := range tableNames { |
There was a problem hiding this comment.
why not extract the common code,
and put the hint type as an input parameter.
| @@ -1800,10 +1812,20 @@ func (b *planBuilder) buildSemiJoin(outerPlan, innerPlan LogicalPlan, onConditio | |||
| if b.TableHints() != nil { | |||
There was a problem hiding this comment.
can this branch be extracted as a function.
There was a problem hiding this comment.
@winoros this branch can be extracted as a function?
There was a problem hiding this comment.
i think it's not too much need to do this.
|
LGTM |
|
/run-all-tests |
|
LGTM |
|
/run-integration-common-test |
|
/run-integration-compatibility-test |
|
@winoros Remind to update the document. |
for #5282
PTAL @hanfei1991 @coocood @lamxTyler