planner, expression: fix simplify outer join with cast

[justarandomstring]

What problem does this PR solve?

Before this PR, left outer join was incorrectly simplified into inner join:

mysql> explain select * from t t1 left join t t2 on t1.a = t2.a where cast(t1.c_str as char) = '2.3';
+--------------------------+----------+-----------+------------------------------------------------------------+
| id                       | count    | task      | operator info                                              |
+--------------------------+----------+-----------+------------------------------------------------------------+
| MergeJoin_8              | 10000.00 | root      | inner join, left key:Column#1, right key:Column#11         |
| ├─Selection_20           | 8000.00  | root      | eq(cast(Column#6), "2.3")                                  |
| │ └─TableReader_22       | 10000.00 | root      | data:TableScan_21                                          |
| │   └─TableScan_21       | 10000.00 | cop[tikv] | table:t1, range:[-inf,+inf], keep order:true, stats:pseudo |
| └─TableReader_24         | 10000.00 | root      | data:TableScan_23                                          |
|   └─TableScan_23         | 10000.00 | cop[tikv] | table:t2, range:[-inf,+inf], keep order:true, stats:pseudo |
+--------------------------+----------+-----------+------------------------------------------------------------+
6 rows in set, 1 warning (0.00 sec)

After this PR:

mysql> explain select * from t t1 left join t t2 on t1.a = t2.a where cast(t1.c_str as char) = '2.3';
+--------------------------+----------+-----------+------------------------------------------------------------+
| id                       | count    | task      | operator info                                              |
+--------------------------+----------+-----------+------------------------------------------------------------+
| MergeJoin_8              | 10000.00 | root      | left outer join, left key:Column#1, right key:Column#11    |
| ├─Selection_19           | 8000.00  | root      | eq(cast(Column#6), "2.3")                                  |
| │ └─TableReader_21       | 10000.00 | root      | data:TableScan_20                                          |
| │   └─TableScan_20       | 10000.00 | cop[tikv] | table:t1, range:[-inf,+inf], keep order:true, stats:pseudo |
| └─TableReader_23         | 10000.00 | root      | data:TableScan_22                                          |
|   └─TableScan_22         | 10000.00 | cop[tikv] | table:t2, range:[-inf,+inf], keep order:true, stats:pseudo |
+--------------------------+----------+-----------+------------------------------------------------------------+
6 rows in set (0.00 sec)

What is changed and how it works?

Handle ScalarFunction Cast in a different logic in EvaluateExprWithNull.

Before this PR, the following code is part of how EvaluateExprWithNull handles ScalarFunction.

NewFunctionInternal(ctx, x.FuncName.L, types.NewFieldType(mysql.TypeTiny), args...)

This line will set the default RetType of ScarlarFunction into mysql.TypeTiny. For those ScalarFunctions other than Cast, RetType can be determined correctly as it will be recalcuated in NewFunctionImpl. However, when handling Cast, newFunctionImpl simply pass RetType to BuildCastFunction without any other processing, which may lead a wrong RetType.

In the example, the Cast is CastStringToIntSig before this PR instead of CastStringToStringSig, which it supposed to be.

// newFunctionImpl creates a new scalar function or constant.
func newFunctionImpl(ctx sessionctx.Context, fold bool, funcName string, retType *types.FieldType, args ...Expression) (Expression, error) {
	if retType == nil {
		return nil, errors.Errorf("RetType cannot be nil for ScalarFunction.")
	}
	if funcName == ast.Cast {
		return BuildCastFunction(ctx, args[0], retType), nil
	}
	fc, ok := funcs[funcName]

Check List

Tests

• Unit test

Code changes

• Has exported function/method change

Side effects

• Change some execute plan

Related changes

• Need to cherry-pick to the release branch

Release note

• fix bug that simplify outer join get wrong result with cast in where clause.

[CLAassistant]

All committers have signed the CLA.

[foreyes]

/run-unit-test

[foreyes]

/run-all-tests

[codecov]

Codecov Report

Merging #12701 into master will decrease coverage by 0.0409%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #12701        +/-   ##
================================================
- Coverage   79.9122%   79.8713%   -0.0409%     
================================================
  Files           462        462                
  Lines        105308     104786       -522     
================================================
- Hits          84154      83694       -460     
  Misses        14906      14906                
+ Partials       6248       6186        -62

[winoros]

Did you try to only change this line to NewFunctionInternal(ctx, x.FuncName.L, x.RetType, args...)?

[justarandomstring]

I've tried, and it can pass all the tests :)
However, according to this line, it is possible that RetType cannot be overwritten by builtinRetTp. I don't know the exact reason why using mysql.TypeTiny as the default type, but in order to reduce the potential risks, I kept the original logic, that is, use mysql.TypeTiny as the default type.

[winoros]

That line won't affect x.RetType.
*retType = *builtinRetTp would affect.

[justarandomstring]

In this scalar function, when f.getRetTp() is mysql.TypeUnspecified, sf.RetType is mysql.TypeTiny, instead of mysql.TypeUnspecified.

If we change this line to NewFunctionInternal(ctx, x.FuncName.L, x.RetType, args...), sf.RetType may not be mysql.TypeTiny anymore when f.getRetTp() is mysql.TypeUnspecified. I'm not sure if this change has bad influence.

[winoros]

This method just subsitute some columns with NULLs. Use original type should not be worse.

[justarandomstring]

Applied, thanks!

[winoros]

lgtm

[eurekaka]

LGTM

[sre-bot]

/run-all-tests

[sre-bot]

cherry pick to release-3.1 in PR #12720

[sre-bot]

cherry pick to release-3.0 in PR #12721

[sre-bot]

cherry pick to release-2.1 failed

[winoros]

@justarandomstring
Hi, sorry for bothering you after the pull request is merged. Would you please make a cherry-pick commit picking this pr's changes to TiDB's release-2.1 branch?

[justarandomstring]

@winoros No problem. I'll do it in a few days.