expression: refactor constant folding for IF&IFNULL

[zz-jason]

Signed-off-by: Jian Zhang zjsariel@gmail.com

What problem does this PR solve?

1. If in strict sql mode, fold ifnull should not log Data Truncated warning messages
2. If in non-strict sql mode, fold ifnull should not return Data Truncated warnings to the client.

What is changed and how it works?

1. Don't directly call Constant.EvalInt(), which results in error if the constant is a string. I changed it to call Constant.Eval() to get a Datum, use that Datum to check whether the first parameter is a NULL value.

2. If the first parameter for IFNULL is a constant and it's not a NULL value, we can directly return that constant parameter. In the old implementation, we returned the original IFNULL expression.

Check List

Tests

• Unit test TODO: add it later Done

Related changes

• May need to cherry-pick it to the release branch, for it increases the usability of TiDB release versions. Can be discussed later.

[zz-jason]

/run-all-tests

[codecov-io]

Codecov Report

Merging #9094 into master will increase coverage by 0.01%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##           master    #9094      +/-   ##
==========================================
+ Coverage   67.15%   67.16%   +0.01%     
==========================================
  Files         372      372              
  Lines       77839    77836       -3     
==========================================
+ Hits        52270    52282      +12     
+ Misses      20896    20890       -6     
+ Partials     4673     4664       -9

Impacted Files	Coverage Δ
expression/constant_fold.go	85.86% <75%> (+2.71%)	⬆️
infoschema/infoschema.go	76.31% <0%> (-1.32%)	⬇️
expression/builtin_control.go	60.74% <0%> (-0.83%)	⬇️
executor/index_lookup_join.go	77.28% <0%> (-0.64%)	⬇️
executor/distsql.go	72.53% <0%> (-0.47%)	⬇️
executor/executor.go	67.31% <0%> (+0.27%)	⬆️
expression/schema.go	94.53% <0%> (+0.78%)	⬆️
ddl/delete_range.go	80.42% <0%> (+6.87%)	⬆️
ddl/session_pool.go	93.1% <0%> (+10.34%)	⬆️
util/systimemon/systime_mon.go	100% <0%> (+20%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc0e70d...72b40b9. Read the comment docs.

[winoros]

Can we check the returned from EvalInt?

[XuHuaiyu]

Don't directly call Constant.EvalInt(), which results in error if the constant is a string.

It seems that it's not correct to call ToInt64 in Constant.EvalInt when a Datum is of type KindString since it goes against the original intent of this function.

[zz-jason]

Don't directly call Constant.EvalInt(), which results in error if the constant is a string.

It seems that it's not correct to call ToInt64 in Constant.EvalInt when a Datum is of type KindString since it goes against the original intent of this function.

If that datum is typed ENUM/BIT/SET, we have to call Datum.ToInt64() to get the correct value.

[eurekaka]

LGTM

[winoros]

will there be unit test in this pr?

[zz-jason]

/run-all-tests

[XuHuaiyu]

Don't directly call Constant.EvalInt(), which results in error if the constant is a string.

It seems that it's not correct to call ToInt64 in Constant.EvalInt when a Datum is of type KindString since it goes against the original intent of this function.

If that datum is typed ENUM/BIT/SET, we have to call Datum.ToInt64() to get the correct value.

if c.GetType().Hybrid() || c.Value.Kind() == types.KindBinaryLiteral || c.Value.Kind() == types.KindString {}

func (ft *FieldType) Hybrid() bool {
	return ft.Tp == mysql.TypeEnum || ft.Tp == mysql.TypeBit || ft.Tp == mysql.TypeSet
}

c.GetType.Hybrid() already checked the it.

[XuHuaiyu]

LGTM