expression: improve constant folding of UDF if & ifnull #6677

lzmhhh123 · 2018-05-29T09:01:52Z

Now, the constant folding of UDF if & ifnull is as follow.

tidb(localhost:4000) > desc select if(1, 10, 0) from ta;
+---------------+--------------+---------------+------+-----------------------------------------------+----------+
| id            | parents      | children      | task | operator info                                 | count    |
+---------------+--------------+---------------+------+-----------------------------------------------+----------+
| TableScan_4   |              |               | cop  | table:ta, range:[-inf,+inf], keep order:false | 10000.00 |
| TableReader_5 | Projection_3 |               | root | data:TableScan_4                              | 10000.00 |
| Projection_3  |              | TableReader_5 | root | 10                                            | 10000.00 |
+---------------+--------------+---------------+------+-----------------------------------------------+----------+
3 rows in set (0.00 sec)

tidb(localhost:4000) > desc select if(1, ta.a, 0) from ta;
+---------------+--------------+---------------+------+-----------------------------------------------+----------+
| id            | parents      | children      | task | operator info                                 | count    |
+---------------+--------------+---------------+------+-----------------------------------------------+----------+
| TableScan_4   |              |               | cop  | table:ta, range:[-inf,+inf], keep order:false | 10000.00 |
| TableReader_5 | Projection_3 |               | root | data:TableScan_4                              | 10000.00 |
| Projection_3  |              | TableReader_5 | root | if(1, root.ta.a, 0)                           | 10000.00 |
+---------------+--------------+---------------+------+-----------------------------------------------+----------+
3 rows in set (0.01 sec)

tidb(localhost:4000) > desc select ifnull(null, ta.a) from ta;
+---------------+--------------+---------------+------+-----------------------------------------------+----------+
| id            | parents      | children      | task | operator info                                 | count    |
+---------------+--------------+---------------+------+-----------------------------------------------+----------+
| TableScan_4   |              |               | cop  | table:ta, range:[-inf,+inf], keep order:false | 10000.00 |
| TableReader_5 | Projection_3 |               | root | data:TableScan_4                              | 10000.00 |
| Projection_3  |              | TableReader_5 | root | ifnull(null, root.ta.a)                       | 10000.00 |
+---------------+--------------+---------------+------+-----------------------------------------------+----------+
3 rows in set (0.00 sec)

The second and third queries are expected to as same as desc select ta.a from ta;.
This pr is to improve this situation.

After this pr changes, it will look like following:

tidb(localhost:4000) > desc select if(1, ta.a, 0) from ta;
+---------------+---------+----------+------+-----------------------------------------------+----------+
| id            | parents | children | task | operator info                                 | count    |
+---------------+---------+----------+------+-----------------------------------------------+----------+
| TableScan_4   |         |          | cop  | table:ta, range:[-inf,+inf], keep order:false | 10000.00 |
| TableReader_5 |         |          | root | data:TableScan_4                              | 10000.00 |
+---------------+---------+----------+------+-----------------------------------------------+----------+
2 rows in set (0.01 sec)

tidb(localhost:4000) > desc select ifnull(null, ta.a) from ta;
+---------------+---------+----------+------+-----------------------------------------------+----------+
| id            | parents | children | task | operator info                                 | count    |
+---------------+---------+----------+------+-----------------------------------------------+----------+
| TableScan_4   |         |          | cop  | table:ta, range:[-inf,+inf], keep order:false | 10000.00 |
| TableReader_5 |         |          | root | data:TableScan_4                              | 10000.00 |
+---------------+---------+----------+------+-----------------------------------------------+----------+
2 rows in set (0.00 sec)

tidb(localhost:4000) > desc select ta.a from ta;
+---------------+---------+----------+------+-----------------------------------------------+----------+
| id            | parents | children | task | operator info                                 | count    |
+---------------+---------+----------+------+-----------------------------------------------+----------+
| TableScan_4   |         |          | cop  | table:ta, range:[-inf,+inf], keep order:false | 10000.00 |
| TableReader_5 |         |          | root | data:TableScan_4                              | 10000.00 |
+---------------+---------+----------+------+-----------------------------------------------+----------+
2 rows in set (0.00 sec)

The queries of desc select if(1, ta.a, 0) from ta; and desc select ifnull(null, ta.a) from ta; are as same as desc select ta.a from ta;.

lzmhhh123 · 2018-05-29T09:10:31Z

@zz-jason @winoros PTAL.

zz-jason · 2018-05-29T12:21:58Z

@XuHuaiyu PTAL

zz-jason · 2018-05-29T12:23:54Z

expression/constant_fold.go

+		case *builtinIfNullDecimalSig:
+			foldedArg0, _ := foldConstant(args[0])
+			if _, conOK := foldedArg0.(*Constant); conOK {
+				_, isNull0, err := args[0].EvalDecimal(ctx, nil)


Can we:

constArg, isConst := foldedArg0.(*Constant); if isConst && constArg.Value.ToBool(sctx) { return foldConstant(args[1]) }

winoros · 2018-05-31T06:01:07Z

expression/constant_fold.go

+		// This switch is for UDF if or ifnull constant folding.
+		// If first arg is constant, the rest args will not be checked whether is constant.
+		var ctx sessionctx.Context
+		if bbf, ok := x.Function.(*baseBuiltinFunc); ok {


you can use x.Function.getCtx(), so avoid one if statement.

zz-jason · 2018-05-31T06:36:45Z

expression/constant_fold.go

+
+		// This switch is for UDF if or ifnull constant folding.
+		// If first arg is constant, the rest args will not be checked whether is constant.
+		switch x.Function.(type) {


can we use x.FuncName.L == ast.If to replace case *builtinIfIntSig ... ?

zz-jason · 2018-05-31T07:42:13Z

expression/constant_fold.go

@@ -23,13 +25,50 @@ func FoldConstant(expr Expression) Expression {
 	return e
 }

+// foldConstant for BuiltinIfSig like BuiltinIfIntSig, BuiltinIfRealSig, BuiltinIfTimeSig and so on.
+func foldConstantForBuiltinIfSig(args []Expression, ctx sessionctx.Context) (Expression, bool) {


How about changing this function name to foldScalarFuncForIF and:

return the folded function if this IF function can be folded to a simper one

return the origin function if this IF function can not be folded.

shenli · 2018-06-04T07:29:04Z

@zz-jason @winoros @XuHuaiyu PTAL

shenli · 2018-06-04T07:29:53Z

/run-all-tests

zz-jason · 2018-06-04T09:11:57Z

expression/constant_fold.go

+		case ast.Ifnull:
+			foldedArg0, _ := foldConstant(args[0])
+			if constArg, isConst := foldedArg0.(*Constant); isConst {
+				valueNotNull, err := constArg.Value.ToBool(nil)


Can we call constArg.EvalInt just like what we do in line 41?

XuHuaiyu · 2018-06-05T07:32:41Z

expression/constant_fold.go

+
+		// This switch is for UDF if or ifnull constant folding.
+		// If first arg is constant, the rest args will not be checked whether is constant.
+		switch x.FuncName.L {


We can define a map like:

var specialFoldHandler = map[string]func(*ScalarFunction)(Expression, bool){ ast.If: ifFoldHandler, ast.Ifnull: ifNullFoldHandler, } func ifFoldHandler(f *ScalarFunction) (Expression, bool) { } func ifNullFoldHandler(f *ScalarFunction) (Expression, bool) { }

then, we check:

f, ok := specialFoldHandler[x.FuncName.L]; ok { return f(x) }

XuHuaiyu · 2018-06-05T07:34:07Z

Plz add some explain test cases.

XuHuaiyu · 2018-06-05T11:48:20Z

expression/constant_fold.go

 func foldConstant(expr Expression) (Expression, bool) {
 	switch x := expr.(type) {
 	case *ScalarFunction:
 		if _, ok := unFoldableFunctions[x.FuncName.L]; ok {
 			return expr, false
 		}
+		// specialFoldHandler stores functions for special UDF to constant flod
+		var specialFoldHandler = map[string]func(*ScalarFunction) (Expression, bool){


put this map outside this function.

I have tried to put it in expression/function_traits.go. But it cause Initialization Loop error.

Not just in expression/function_traits.go. In any place outside function will cause initialization loop.

var specialFoldHandler = map[string]func(*ScalarFunction) (Expression, bool){} func init() { // specialFoldHandler stores functions for special UDF to constant fold specialFoldHandler = map[string]func(*ScalarFunction) (Expression, bool){ ast.If: ifFoldHandler, ast.Ifnull: ifNullFoldHandler, } }

XuHuaiyu · 2018-06-05T11:49:17Z

expression/constant_fold.go

+		}
+		return foldConstant(args[2])
+	}
+	isDeferredConst := false


var isDeferred, isDeferredConst bool

XuHuaiyu · 2018-06-05T11:51:03Z

expression/constant_fold.go

 func foldConstant(expr Expression) (Expression, bool) {
 	switch x := expr.(type) {
 	case *ScalarFunction:
 		if _, ok := unFoldableFunctions[x.FuncName.L]; ok {
 			return expr, false
 		}
+		// specialFoldHandler stores functions for special UDF to constant flod


s/ flod/ fold

XuHuaiyu · 2018-06-06T09:00:34Z

LGTM
PTAL @zz-jason

zz-jason

LGTM

zz-jason · 2018-06-06T09:13:34Z

/run-all-tests

plan: improve UDF if & ifnull constant folding

282bdac

lzmhhh123 changed the title ~~plan: improve constant folding of UDF if & ifnull~~ expression: improve constant folding of UDF if & ifnull May 29, 2018

zz-jason added the type/enhancement label May 29, 2018

zz-jason reviewed May 29, 2018

View reviewed changes

address comment

12e2917

winoros reviewed May 31, 2018

View reviewed changes

address comment

b06e116

zz-jason reviewed May 31, 2018

View reviewed changes

address comment

080650f

zz-jason reviewed May 31, 2018

View reviewed changes

address comment

2e319d8

Merge branch 'master' into dev/ifconstantfold

e8756b0

zz-jason reviewed Jun 4, 2018

View reviewed changes

address comment

41ec1e8

XuHuaiyu reviewed Jun 5, 2018

View reviewed changes

address comment & add some explaintest

5003c38

XuHuaiyu reviewed Jun 5, 2018

View reviewed changes

lzmhhh123 added 2 commits June 5, 2018 20:05

address comments

536ed2b

put specialFoldFunction map out

fb4e4e0

zz-jason approved these changes Jun 6, 2018

View reviewed changes

Merge branch 'master' into dev/ifconstantfold

98fecfc

zz-jason added the status/LGT2 Indicates that a PR has LGTM 2. label Jun 6, 2018

Merge branch 'master' into dev/ifconstantfold

3b41979

zz-jason added the all-tests-passed label Jun 6, 2018

Merge branch 'master' into dev/ifconstantfold

7d3546b

XuHuaiyu merged commit 66c4112 into pingcap:master Jun 7, 2018

lzmhhh123 deleted the dev/ifconstantfold branch July 24, 2019 05:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expression: improve constant folding of UDF if & ifnull #6677

expression: improve constant folding of UDF if & ifnull #6677

lzmhhh123 commented May 29, 2018 •

edited

lzmhhh123 commented May 29, 2018

zz-jason commented May 29, 2018

zz-jason May 29, 2018

winoros May 31, 2018 •

edited

zz-jason May 31, 2018

zz-jason May 31, 2018

shenli commented Jun 4, 2018

shenli commented Jun 4, 2018

zz-jason Jun 4, 2018

XuHuaiyu Jun 5, 2018

XuHuaiyu commented Jun 5, 2018

XuHuaiyu Jun 5, 2018

lzmhhh123 Jun 5, 2018

lzmhhh123 Jun 5, 2018

XuHuaiyu Jun 5, 2018

XuHuaiyu Jun 5, 2018

XuHuaiyu Jun 5, 2018

XuHuaiyu commented Jun 6, 2018

zz-jason left a comment

zz-jason commented Jun 6, 2018

expression: improve constant folding of UDF if & ifnull #6677

expression: improve constant folding of UDF if & ifnull #6677

Conversation

lzmhhh123 commented May 29, 2018 • edited

lzmhhh123 commented May 29, 2018

zz-jason commented May 29, 2018

Choose a reason for hiding this comment

winoros May 31, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shenli commented Jun 4, 2018

shenli commented Jun 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XuHuaiyu commented Jun 5, 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

XuHuaiyu commented Jun 6, 2018

zz-jason left a comment

Choose a reason for hiding this comment

zz-jason commented Jun 6, 2018

lzmhhh123 commented May 29, 2018 •

edited

winoros May 31, 2018 •

edited