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: support explain and explain analyze statements for prepared queries #23725

Open
qw4990 opened this issue Mar 31, 2021 · 5 comments
Open
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. sig/planner SIG: Planner type/enhancement The issue or PR belongs to an enhancement. type/feature-request Categorizes issue or PR as related to a new feature.

Comments

@qw4990
Copy link
Contributor

qw4990 commented Mar 31, 2021

Feature Request

Is your feature request related to a problem? Please describe:

Explain and explain analyze statements can print query plans, which is useful when optimizing queries and investigating issues, but prepared queries cannot support this feature:

mysql> explain execute stmt using @a,@b,@c;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use line 1 column 26 near "using @a,@b,@c"

Describe the feature you'd like:

Support explain and explain analyze statements for prepared queries, for example:

mysql> explain execute stmt using @a,@b,@c;
+--------------------------+---------+-----------+---------------+---------------------------------------------------------+
| id                       | estRows | task      | access object | operator info                                           |
+--------------------------+---------+-----------+---------------+---------------------------------------------------------+
| TableReader_7            | 0.04    | root      |               | data:Selection_6                                        |
| └─Selection_6            | 0.04    | cop[tikv] |               | ge(test.ta.a, "a"), ge(test.ta.b, 2), le(test.ta.b, 10) |
|   └─TableRangeScan_5     | 1.67    | cop[tikv] | table:ta      | range:["a",+inf], keep order:false, stats:pseudo        |
+--------------------------+---------+-----------+---------------+---------------------------------------------------------+
3 rows in set (0.63 sec)

Describe alternatives you've considered:

To get plans of prepared statements, now we can use explain for {ConnID} or search them from slow log file, but both these two methods are not convenient.

Teachability, Documentation, Adoption, Migration Strategy:

@qw4990 qw4990 added type/enhancement The issue or PR belongs to an enhancement. sig/planner SIG: Planner type/feature-request Categorizes issue or PR as related to a new feature. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Mar 31, 2021
@qw4990
Copy link
Contributor Author

qw4990 commented Apr 1, 2021

Any options about this requirement? @zz-jason @eurekaka @winoros
Since it is not complex and urgent, we can ask some contributors to implement this feature if it is accepted.

@LaPetiteSouris
Copy link

LaPetiteSouris commented May 31, 2022

@qw4990 Question from a new comer here.

I tried to take a look into parser.y and it seems that we may have a few solutions

  1. Either add ExecuteStmt into ExplainableStmt. This looks neats, but not sure if it is valid, as not all executable statements are explainable. Moreover, there are tons of shift/reduce conflicts to deal with in this case.

  2. Do a customize:

  |	ExplainSym "ANALYZE" ExecuteStmt
	  {
		  $$ = &ast.ExplainStmt{
			  Stmt:    $3,
			  Format:  "row",
			  Analyze: true,
		  }
	  }
  

OR


|	ExplainSym ExecuteStmt
	{
		$$ = &ast.ExplainStmt{
			Stmt:   $2,
			Format: "row",
		}
	}

This approach works, however, it will duplicate a bit of code in parse.y

Another question since I do not totally understand:

ExplainSym "ANALYZE" ExecuteStmt does not generate any shift/reduce conflict, but ExplainSym ExecuteStmt does. Do you have any suggestion ? I am quite blocked in this point, there must be something I miss. My best guess is that there is something to do with the Identifier argument in the definition of ExecuteStmt

ExecuteStmt:
	"EXECUTE" Identifier
	{
		$$ = &ast.ExecuteStmt{Name: $2}
	}

It seems that adding any valid keyword between ExplainSym and ExecuteStmt solves the shift/conflict problem (which is the case of ExplainSym "ANALYZE" ExecuteStmt. But I still do not understand why.

@LaPetiteSouris
Copy link

ExplainSym "ANALYZE" ExecuteStmt does not generate any shift/reduce conflict, but ExplainSym ExecuteStmt does. Do you have any suggestion ? I am quite blocked in this point, there must be something I miss. My best guess is that there is something to do with the Identifier argument in the definition of ExecuteStmt

ExecuteStmt:
	"EXECUTE" Identifier
	{
		$$ = &ast.ExecuteStmt{Name: $2}
	}

It seems that adding any valid keyword between ExplainSym and ExecuteStmt solves the shift/conflict problem (which is the case of ExplainSym "ANALYZE" ExecuteStmt. But I still do not understand why.

It seems that ExplainSym ExecuteStmt is ambiguous , as it is translated to EXPLAIN EXECUTE Identifier and Identifier can be anything from reserved to non reserved keyword, which is indeed ambiguous . I will try to address this issue in the PR

@valkyr13
Copy link

can i pick this up?

@LaPetiteSouris
Copy link

can i pick this up?

I don't have time to continue any further unfortunately, so feel free to grab. No objection for my side .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. sig/planner SIG: Planner type/enhancement The issue or PR belongs to an enhancement. type/feature-request Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants