Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

planner: add EXPLAIN FORMAT=JSON #20482

Closed
wants to merge 17 commits into from
Closed

planner: add EXPLAIN FORMAT=JSON #20482

wants to merge 17 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 16, 2020

What problem does this PR solve?

Issue Number: Fixes #20378

What is changed and how it works?

What's Changed:

This implements EXPLAIN FORMAT=JSON.

Because the operatorInfo is built by concatenating strings at a much lower level (planner/core/explain.go:37 - dataAccessor interface), it is just returned as a string (not ideal). I have forked #20520 to convert the delimiter to a ;.

Here is an example:

*************************** 1. row ***************************
EXPLAIN: {
  "id": "HashJoin_17",
  "estRows": 12487.5,
  "task": "",
  "accessObject": "",
  "operatorInfo": "inner join, equal:[eq(test.t2.t1_id, test.t1.id)]",
  "children": [
    {
      "id": "TableReader_20(Build)",
      "estRows": 9990,
      "task": "",
      "accessObject": "",
      "operatorInfo": "data:Selection_19",
      "children": [
        {
          "id": "Selection_19",
          "estRows": 9990,
          "task": "",
          "accessObject": "",
          "operatorInfo": "not(isnull(test.t2.t1_id))",
          "children": [
            {
              "id": "TableFullScan_18",
              "estRows": 10000,
              "task": "",
              "accessObject": "table:t2",
              "operatorInfo": "keep order:false, stats:pseudo",
              "children": null
            }
          ]
        }
      ]
    },
    {
      "id": "TableReader_22(Probe)",
      "estRows": 10000,
      "task": "",
      "accessObject": "",
      "operatorInfo": "data:TableFullScan_21",
      "children": [
        {
          "id": "TableFullScan_21",
          "estRows": 10000,
          "task": "",
          "accessObject": "table:t1",
          "operatorInfo": "keep order:false, stats:pseudo",
          "children": null
        }
      ]
    }
  ]
}

1 row in set (0.00 sec)

Related changes

Check List

Tests

  • Unit tests

Side effects

  • None

Release note

  • The EXPLAIN statement now supports the optional FORMAT=JSON syntax.

@ghost
Copy link
Author

ghost commented Oct 16, 2020

@breeswish @zz-jason may I ask your opinion on these two questions in the description?

@breezewish
Copy link
Member

breezewish commented Oct 16, 2020

(1) The golang JSON encoder will by default escape HTML characters. I think this creates ugly output in operatorInfo

Agree. Golang JSON encoder is escaped for HTML purpose and we don't need it.

(2) The operatorInfo is built by concatenating strings at a much lower level (planner/core/explain.go:37 - dataAccessor interface)

We should change it (in future PRs). It is also the foundation of enabling more data to be visualized in TiDB Dashboard, for example, an interactive and nice plan view from slow query and statement page. My own preference is to use Protobuf, whose structure is JSON serializable and also allow TiDB Dashboard to parse binary directly. @crazycs520 PTAL, we have talked about this previously.

@zz-jason
Copy link
Member

zz-jason commented Oct 19, 2020

(1) The golang JSON encoder will by default escape HTML characters. I think this creates ugly output in operatorInfo, and suggest we disable it.

Agree.

The best solution I have is to change concatenation between info items to use the semicolon character ;, but this would be an incompatible change that also affects other explain formats.

@qw4990 do you have any idea?

@zz-jason zz-jason requested review from qw4990 and winoros October 19, 2020 07:35
@qw4990
Copy link
Contributor

qw4990 commented Oct 19, 2020

The best solution I have is to change concatenation between info items to use the semicolon character ; , but this would be an incompatible change that also affects other explain formats.

I agree with it. Actually, I also meet this problem in another project where I want to parse and compare plans.

@ghost ghost marked this pull request as ready for review October 19, 2020 18:12
@ghost ghost self-requested a review as a code owner October 19, 2020 18:12
@ghost
Copy link
Author

ghost commented Oct 19, 2020

This is now ready to review. The tests are just failing waiting on the parser PR to merge.

@ghost ghost added the sig/planner SIG: Planner label Oct 20, 2020
planner/core/common_plans.go Outdated Show resolved Hide resolved
planner/core/common_plans.go Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Oct 22, 2020

/run-all-tests

@qw4990 qw4990 self-requested a review October 27, 2020 11:59
@qw4990 qw4990 added the sig/execution SIG execution label Oct 30, 2020
Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 2, 2020
@qw4990 qw4990 removed the status/LGT1 Indicates that a PR has LGTM 1. label Nov 2, 2020
tk.MustExec("CREATE TABLE t2 (id INT NOT NULL PRIMARY KEY, t1_id INT)")

var js core.JSONOperatorRow
result := tk.MustQuery("EXPLAIN FORMAT=JSON SELECT t1.* FROM t1 INNER JOIN t2 ON t1.id=t2.t1_id")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add more cases to cover more operators.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qw4990 I was worried about test stability as statistics and operator auto-naming can change. What I arrived at, was to use a regular expression. PTAL again, thx :-)

@ghost ghost added the status/PTAL label Nov 10, 2020
@ti-srebot
Copy link
Contributor

@qw4990, @winoros, PTAL.

2 similar comments
@ti-srebot
Copy link
Contributor

@qw4990, @winoros, PTAL.

@ti-srebot
Copy link
Contributor

@qw4990, @winoros, PTAL.

@ghost
Copy link
Author

ghost commented Dec 15, 2020

We can revisit this when #20520 is complete.

@ghost ghost closed this Dec 15, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution sig/planner SIG: Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support explain format=json
4 participants