expression: Add warning info for exprs that can not be pushed to storage layer #22713

windtalker · 2021-02-04T02:43:52Z

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:

Currently, when user running a query, and found the query is slower than expected because some part of the query(e.g. Selection or Aggregation) is not pushed down to coprocessor, there is no easy way to find out which function not supported by coprocessor.

What is changed and how it works?

Proposal: xxx

What's Changed:

How it Works:

According to Mysql document:

The EXPLAIN statement produces extra (“extended”) information that is not part of EXPLAIN output but can be viewed by issuing a SHOW WARNINGS statement following EXPLAIN. As of MySQL 8.0.12, extended information is available for SELECT, DELETE, INSERT, REPLACE, and UPDATE statements. Prior to 8.0.12, extended information is available only for SELECT statements.
The Message value in SHOW WARNINGS output displays how the optimizer qualifies table and column names in the SELECT statement, what the SELECT looks like after the application of rewriting and optimization rules, and possibly other notes about the optimization process.

It is reasonable to add warnings about functions that can not be pushed to coprocessor when user executing an explain statement, so this pr add this information to make user easier to figure out why Selection or Aggregation is not pushed to coprocessor.

Related changes

Need to cherry-pick to the release branch

Check List

Tests

Integration test
Manual test (add detailed scripts or steps below)

Side effects

Release note

Add warning info for exprs that can not be pushed to storage layer for explain statement.

…er for explain/explain analyze statement

…ning_info

ilovesoup · 2021-02-04T02:49:22Z

expression/expression.go

+			if storeType == kv.UnSpecified {
+				storageName = "storage layer"
+			}
+			pc.sc.AppendWarning(errors.New("Scalar function '" + scalarFunc.FuncName.L + "'(signature: " + scalarFunc.Function.PbCode().String() + ") can not be pushed to " + storageName))


Looks like each time only one function can be shown. For a real world case it will be tricky.

In TiDB implementation, for each DNF expression, If a function can not be pushed down, then all the other functions in the same DNF expression will not be checked, as far as I known, TiDB will convert filter expression into a list of DNF before predicate pushdown, so if function_1 and function_2 is not supported by the storage layer, for query:
select * from a where function_1 or function_2, only function_1 will be recorded.
but for query
select * from a where function_1 and function_2, both function_1 and function_2 will be recorded.

why not reading the explain result to find which functions are not pushed down?

windtalker · 2021-02-04T04:26:00Z

/run-all-tests

hanfei1991

LGTM

fzhedu · 2021-02-04T14:15:50Z

expression/expression.go

@@ -1161,6 +1161,13 @@ func canScalarFuncPushDown(scalarFunc *ScalarFunction, pc PbConverter, storeType

 	// Check whether this function can be pushed.
 	if !canFuncBePushed(scalarFunc, storeType) {
+		if pc.sc.InExplainStmt {


for example: select fun1(), agg_fun2() from t where func3() group by c having fun4(); if funx() cannot be pushed down, will this shows 4 warnings, even though it first encounters func3()?

This pr only record the unsupported functions, it does not change the behavior of optimizer, so if func3 is not supported, and the optimizer decide not pushdown agg to the storage layer, then only func3 will be recorded.

other unsupported functions also will be recorded?

fzhedu · 2021-02-04T14:16:51Z

expression/expression.go

@@ -1184,6 +1191,9 @@ func canScalarFuncPushDown(scalarFunc *ScalarFunction, pc PbConverter, storeType

 func canExprPushDown(expr Expression, pc PbConverter, storeType kv.StoreType) bool {
 	if storeType == kv.TiFlash && expr.GetType().Tp == mysql.TypeDuration {
+		if pc.sc.InExplainStmt {


duration and func() will rusult two warnings? if func() cannot be pushed down

If the expr contains Duration, it will return directly without checking function in this expr

…ning_info

zz-jason · 2021-02-18T01:30:32Z

planner/core/task.go

+			if sc.InExplainStmt {
+				storageName := storeType.Name()
+				if storeType == kv.UnSpecified {
+					storageName = "storage layer"


when will we encounter this branch?

In predicate_push_down rule, it uses Unspecified to make sure it can pushdown all the possible expressions, and when generating the cop task, the DataSource will use the specific store type(TiFlash/TiKV/TiDB) to make sure only pushdown supported expressions to each store.

zz-jason · 2021-02-18T01:33:34Z

expression/expression.go

+			if storeType == kv.UnSpecified {
+				storageName = "storage layer"
+			}
+			pc.sc.AppendWarning(errors.New("Scalar function '" + scalarFunc.FuncName.L + "'(signature: " + scalarFunc.Function.PbCode().String() + ") can not be pushed to " + storageName))


why not reading the explain result to find which functions are not pushed down?

windtalker · 2021-02-18T01:47:56Z

Explain result only tells that Filter/Agg is not pushed down to cop, it can not answer the question "why filter/agg is not pushed down". For example:

if the filter is func1 and func2, and it is not pushed down, user does not have a convenient way to tell whether func1 or func2 is not supported in storage layer.
if an agg is not pushed down, user does not have a convenient way to tell whether the agg is not pushed to storage layer because some of the expressions in agg is not supported in storage layer or just because the CBO think it is not worth pushing down the agg to storage layer.

fzhedu

LGTM

fzhedu · 2021-02-19T14:58:31Z

/merge

ti-srebot · 2021-02-19T15:00:55Z

/run-all-tests

ti-srebot · 2021-02-19T15:03:25Z

@windtalker merge failed.

windtalker · 2021-02-20T01:14:13Z

/run-integration-copr-test

fzhedu · 2021-02-20T02:28:55Z

/merge

ti-srebot · 2021-02-20T02:29:24Z

/run-all-tests

windtalker · 2021-03-01T07:11:44Z

/label needs-cherry-pick-4.0

Rustin170506 · 2021-03-01T07:18:54Z

/label needs-cherry-pick-4.0

Rustin170506 · 2021-03-01T07:21:40Z

/remove-label needs-cherry-pick-4.0

windtalker · 2021-03-01T07:23:40Z

/label needs-cherry-pick-4.0

Signed-off-by: ti-srebot <[email protected]>

ti-srebot · 2021-03-01T07:24:25Z

cherry pick to release-4.0 in PR #23020

…age layer (#22713) (#23020)

windtalker added 3 commits February 4, 2021 09:55

record warnings about functions that can not be pushed to storage lay…

f6f885c

…er for explain/explain analyze statement

add tests

32f386a

fmt code

316c83f

windtalker requested review from a team as code owners February 4, 2021 02:43

windtalker requested review from wshwsh12 and winoros and removed request for a team February 4, 2021 02:43

Merge branch 'master' of https://github.com/pingcap/tidb into add_war…

7e03fd5

…ning_info

ilovesoup reviewed Feb 4, 2021

View reviewed changes

github-actions bot added component/expression sig/execution SIG execution labels Feb 4, 2021

windtalker added 2 commits February 4, 2021 12:22

fix ci

29fdcec

fix

3e441da

fix ci

f9f6927

hanfei1991 reviewed Feb 4, 2021

View reviewed changes

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 4, 2021

fzhedu reviewed Feb 4, 2021

View reviewed changes

Merge branch 'master' of https://github.com/pingcap/tidb into add_war…

5f9e0da

…ning_info

zz-jason reviewed Feb 18, 2021

View reviewed changes

fzhedu approved these changes Feb 19, 2021

View reviewed changes

ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Feb 19, 2021

ti-srebot approved these changes Feb 19, 2021

View reviewed changes

ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Feb 19, 2021

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 19, 2021

Merge branch 'master' into add_warning_info

fecfbf2

ti-srebot merged commit 3ad7f40 into pingcap:master Feb 20, 2021

windtalker mentioned this pull request Feb 23, 2021

expression: log functions that can not be pushed to cop #21977

Closed

ti-chi-bot added the needs-cherry-pick-4.0 label Mar 1, 2021

ti-chi-bot removed the needs-cherry-pick-4.0 label Mar 1, 2021

ti-chi-bot added the needs-cherry-pick-4.0 label Mar 1, 2021

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Mar 1, 2021

cherry pick pingcap#22713 to release-4.0

025fbd6

Signed-off-by: ti-srebot <[email protected]>

ti-srebot mentioned this pull request Mar 1, 2021

expression: Add warning info for exprs that can not be pushed to storage layer (#22713) #23020

Merged

ti-chi-bot pushed a commit that referenced this pull request Mar 1, 2021

expression: Add warning info for exprs that can not be pushed to stor…

8406e7f

…age layer (#22713) (#23020)

windtalker deleted the add_warning_info branch May 21, 2021 01:38

windtalker mentioned this pull request Jun 18, 2021

planner: Log warnings when agg function can not be pushdown in explain statement #25553

Merged

This was referenced Jun 24, 2021

planner: Log warnings when agg function can not be pushdown in explain statement (#25553) #25736

Merged

planner: Log warnings when agg function can not be pushdown in explain statement (#25553) #25737

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expression: Add warning info for exprs that can not be pushed to storage layer #22713

expression: Add warning info for exprs that can not be pushed to storage layer #22713

windtalker commented Feb 4, 2021

ilovesoup Feb 4, 2021

windtalker Feb 4, 2021

zz-jason Feb 18, 2021

windtalker commented Feb 4, 2021

hanfei1991 left a comment

fzhedu Feb 4, 2021

windtalker Feb 5, 2021

fzhedu Feb 19, 2021

fzhedu Feb 4, 2021

windtalker Feb 5, 2021

zz-jason Feb 18, 2021

windtalker Feb 18, 2021

zz-jason Feb 18, 2021

windtalker commented Feb 18, 2021

fzhedu left a comment

fzhedu commented Feb 19, 2021

ti-srebot commented Feb 19, 2021

ti-srebot commented Feb 19, 2021

windtalker commented Feb 20, 2021

fzhedu commented Feb 20, 2021

ti-srebot commented Feb 20, 2021

windtalker commented Mar 1, 2021

Rustin170506 commented Mar 1, 2021

Rustin170506 commented Mar 1, 2021

windtalker commented Mar 1, 2021

ti-srebot commented Mar 1, 2021

expression: Add warning info for exprs that can not be pushed to storage layer #22713

expression: Add warning info for exprs that can not be pushed to storage layer #22713

Conversation

windtalker commented Feb 4, 2021

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

windtalker commented Feb 4, 2021

hanfei1991 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

windtalker commented Feb 18, 2021

fzhedu left a comment

Choose a reason for hiding this comment

fzhedu commented Feb 19, 2021

ti-srebot commented Feb 19, 2021

ti-srebot commented Feb 19, 2021

windtalker commented Feb 20, 2021

fzhedu commented Feb 20, 2021

ti-srebot commented Feb 20, 2021

windtalker commented Mar 1, 2021

Rustin170506 commented Mar 1, 2021

Rustin170506 commented Mar 1, 2021

windtalker commented Mar 1, 2021

ti-srebot commented Mar 1, 2021