planner: support ScalarSubQuery to display them in EXPLAIN #45252

winoros · 2023-07-09T18:09:36Z

What problem does this PR solve?

Issue Number: close #22076

Problem Summary:

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

hawkingrei · 2023-07-10T02:21:30Z

/test all

hawkingrei · 2023-07-10T02:26:05Z

/retest

hawkingrei · 2023-07-10T07:59:29Z

/test all

hawkingrei · 2023-07-12T01:10:11Z

/retest

chrysan · 2023-07-18T17:01:56Z

/retest

chrysan · 2023-07-18T17:05:34Z

planner/core/scalar_subq_expression.go

+
+// EvalInt returns the int64 representation of expression.
+func (*ScalarSubQueryExpr) EvalInt(_ sessionctx.Context, _ chunk.Row) (val int64, isNull bool, err error) {
+	return 0, false, errors.Errorf("Evaluation methods is not implemented for ScalarSubQueryExpr")


Do we need to implement these funcs in the future?

Don't need it for now. Since it will not be executed during optimization.

Need to revisit and implement those EvalXXX later, when we need to support execution under ScalarSubqueryExpr. Please add some comments here.

hawkingrei · 2023-07-18T17:30:23Z

planner/core/scalar_subq_expression.go

+)
+
+// ScalarSubQueryExpr is a expression placeholder for the non-correlated scalar subqueries which can be evaluated during optimizing phase.
+type ScalarSubQueryExpr struct {


core has too many codes in it. so can you put this code into the sub package of the core.

Decoupling the codes in core is not easy work...
And you'll see that the struct embeds basePlan in it, which means we cannot move it out from the package...

I have tried to decouple some codes in core before. But it's really a time costing work.

planner/core/casetest/physicalplantest/physical_plan_test.go

build/BUILD.bazel

sessionctx/variable/tidb_vars.go

executor/executor.go

elsa0520 · 2023-07-19T08:37:17Z

planner/core/casetest/physicalplantest/testdata/plan_suite_in.json

+        "SQL": "explain analyze format = 'brief' select * from t1 where (a, b) = (select a, b from t2 limit 1)",
+        "IsExplainAnalyze": true,
+        "HasErr": false
+      }


Please add some tests about constant folding

sessionctx/variable/session.go

planner/core/expression_rewriter.go

planner/core/scalar_subq_expression.go

util/plancodec/id.go

codecov · 2023-07-19T17:28:01Z

Codecov Report

Merging #45252 (f35bd77) into master (c6d83d7) will increase coverage by 0.0757%.
The diff coverage is 45.7875%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #45252        +/-   ##
================================================
+ Coverage   73.2767%   73.3524%   +0.0757%     
================================================
  Files          1264       1268         +4     
  Lines        389025     391061      +2036     
================================================
+ Hits         285065     286853      +1788     
- Misses        85717      85873       +156     
- Partials      18243      18335        +92

Flag	Coverage Δ
integration	`78.1388% <ø> (?)`
unit	`73.3441% <45.7875%> (+0.0674%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`54.0444% <ø> (ø)`
parser	`85.0554% <ø> (-0.0346%)`	⬇️
br	`52.1557% <ø> (+0.0424%)`	⬆️

hawkingrei · 2023-07-20T02:10:04Z

/retest

elsa0520

LGTM

AilinKid

Rest LGTM

AilinKid · 2023-07-10T09:46:38Z

planner/core/flat_plan.go

+	for _, scalarSubQ := range p.SCtx().GetSessionVars().MapScalarSubQ {
+		castedScalarSubQ, ok := scalarSubQ.(*ScalarSubQueryExpr)
+		if !ok {
+			continue


how can we assert fail here? the log is better to notify at debug level

AilinKid · 2023-07-20T09:09:32Z

planner/core/casetest/scalarsubquery/testdata/plan_suite_out.json

+          "Selection 8000.00 root  ScalarQueryCol#15",
+          "└─TableReader 10000.00 root  data:TableFullScan",
+          "  └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo",
+          "ScalarSubQuery N/A root  Output: ScalarQueryCol#13",


may we follow the dependency order here?

main (use col2 inside) dependency2 (use col1 inside) output col2 dependency1 output col1

We can left for future optimization

AilinKid · 2023-07-20T09:15:22Z

planner/core/casetest/scalarsubquery/testdata/plan_suite_out.json

+      {
+        "SQL": "explain format = 'brief' select * from t1 where (a, b) = (select a, b from t2 limit 1)",
+        "Plan": [
+          "Selection 8000.00 root  eq(test.t1.a, ScalarQueryCol#9), eq(test.t1.b, ScalarQueryCol#10)",


may we add one more case like:
select sub-q as yy, xx, xx from t where yy > 0 order by yy;
this kind subq's output col will be built and used through projection then selection, then sort

AilinKid · 2023-07-20T09:51:55Z

planner/core/scalar_subq_expression.go

+		return s.Value, s.evalErr
+	}
+	err := s.selfEvaluate()
+	if err != nil {


may merge the below two line together?

ti-chi-bot · 2023-07-22T03:25:07Z

[LGTM Timeline notifier]

Timeline:

2023-07-20 08:02:59.11207413 +0000 UTC m=+1183070.882412843: ☑️ agreed by elsa0520.
2023-07-22 03:25:06.188455686 +0000 UTC m=+1339197.958794393: ☑️ agreed by chrysan.

hawkingrei · 2023-07-24T03:21:46Z

/hold

hawkingrei · 2023-07-24T03:22:25Z

/hold

Waiting for the configure review's approval.

hawkingrei · 2023-07-24T03:24:55Z

/unhold

ti-chi-bot · 2023-07-24T03:25:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AilinKid, chrysan, elsa0520, hawkingrei, qw4990

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [AilinKid,chrysan,hawkingrei,qw4990]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

hawkingrei · 2023-07-24T05:19:56Z

/retest

ti-chi-bot · 2023-07-24T06:48:20Z

In response to a cherrypick label: new pull request created to branch release-5.4: #45521.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

seiya-annie · 2024-06-13T10:13:49Z

/found community

planner: support ScalarSubQuery to display them in EXPLAIN

6b296ce

winoros requested a review from a team as a code owner July 9, 2023 18:09

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 9, 2023

winoros mentioned this pull request Jul 9, 2023

subquery execution should be displayed in explain and explain analyze #22076

Closed

ti-chi-bot bot deleted a comment from ti-chi-bot Jul 9, 2023

fix make bazel_build

8167857

winoros added the sig/planner SIG: Planner label Jul 9, 2023

don't affect explain analyze and add tests

6e2f0ff

chrysan reviewed Jul 18, 2023

View reviewed changes

hawkingrei reviewed Jul 18, 2023

View reviewed changes

planner/core/casetest/physicalplantest/physical_plan_test.go Outdated Show resolved Hide resolved

chrysan requested review from fixdb and elsa0520 July 19, 2023 03:35

elsa0520 reviewed Jul 19, 2023

View reviewed changes

build/BUILD.bazel Outdated Show resolved Hide resolved

sessionctx/variable/tidb_vars.go Outdated Show resolved Hide resolved

executor/executor.go Show resolved Hide resolved

elsa0520 reviewed Jul 19, 2023

View reviewed changes

winoros added 4 commits July 19, 2023 21:51

Merge branch 'master' into add-scalar-subq-expr

f9ca88b

fix test

a614994

support multiple returned values

0913496

add filepermission back

85e75b3

elsa0520 approved these changes Jul 20, 2023

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jul 20, 2023

AilinKid reviewed Jul 20, 2023

View reviewed changes

address comments

f35bd77

chrysan approved these changes Jul 22, 2023

View reviewed changes

ti-chi-bot bot added lgtm approved and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 22, 2023

AilinKid approved these changes Jul 22, 2023

View reviewed changes

AilinKid approved these changes Jul 24, 2023

View reviewed changes

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 24, 2023

qw4990 approved these changes Jul 24, 2023

View reviewed changes

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 24, 2023

hawkingrei approved these changes Jul 24, 2023

View reviewed changes

ti-chi-bot bot merged commit 2eb698c into pingcap:master Jul 24, 2023

chrysan added type/5.4-cherry-pick needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. and removed type/5.4-cherry-pick labels Jul 24, 2023

ti-chi-bot mentioned this pull request Jul 24, 2023

planner: support ScalarSubQuery to display them in EXPLAIN (#45252) #45521

Closed

12 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jul 24, 2023

This is an automated cherry-pick of pingcap#45252

83f682b

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

winoros deleted the add-scalar-subq-expr branch July 24, 2023 12:13

winoros mentioned this pull request Jul 25, 2023

optimizer: add docs for shared cte and explain enhancement pingcap/docs-cn#14634

Merged

17 tasks

ti-chi-bot bot added the report/community The community has encountered this bug. label Jun 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner: support ScalarSubQuery to display them in EXPLAIN #45252

planner: support ScalarSubQuery to display them in EXPLAIN #45252

winoros commented Jul 9, 2023 •

edited

Loading

hawkingrei commented Jul 10, 2023

hawkingrei commented Jul 10, 2023

hawkingrei commented Jul 10, 2023

hawkingrei commented Jul 12, 2023

chrysan commented Jul 18, 2023

chrysan Jul 18, 2023

winoros Jul 19, 2023

chrysan Jul 20, 2023

hawkingrei Jul 18, 2023

winoros Jul 19, 2023

elsa0520 Jul 19, 2023

codecov bot commented Jul 19, 2023 •

edited

Loading

hawkingrei commented Jul 20, 2023

elsa0520 left a comment

AilinKid left a comment

AilinKid Jul 10, 2023

AilinKid Jul 20, 2023

winoros Jul 21, 2023

AilinKid Jul 20, 2023

AilinKid Jul 20, 2023

ti-chi-bot bot commented Jul 22, 2023

hawkingrei commented Jul 24, 2023

hawkingrei commented Jul 24, 2023

hawkingrei commented Jul 24, 2023

ti-chi-bot bot commented Jul 24, 2023

hawkingrei commented Jul 24, 2023

ti-chi-bot commented Jul 24, 2023

seiya-annie commented Jun 13, 2024

planner: support ScalarSubQuery to display them in EXPLAIN #45252

planner: support ScalarSubQuery to display them in EXPLAIN #45252

Conversation

winoros commented Jul 9, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

hawkingrei commented Jul 10, 2023

hawkingrei commented Jul 10, 2023

hawkingrei commented Jul 10, 2023

hawkingrei commented Jul 12, 2023

chrysan commented Jul 18, 2023

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

codecov bot commented Jul 19, 2023 • edited Loading

Codecov Report

hawkingrei commented Jul 20, 2023

elsa0520 left a comment

Choose a reason for hiding this comment

AilinKid 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

ti-chi-bot bot commented Jul 22, 2023

[LGTM Timeline notifier]

hawkingrei commented Jul 24, 2023

hawkingrei commented Jul 24, 2023

hawkingrei commented Jul 24, 2023

ti-chi-bot bot commented Jul 24, 2023

hawkingrei commented Jul 24, 2023

ti-chi-bot commented Jul 24, 2023

seiya-annie commented Jun 13, 2024

winoros commented Jul 9, 2023 •

edited

Loading

codecov bot commented Jul 19, 2023 •

edited

Loading