parser: support query block in hint#491
Conversation
| // HintTable is table in the hint. It may have query block info. | ||
| type HintTable struct { | ||
| TableName model.CIStr | ||
| QBName model.CIStr |
There was a problem hiding this comment.
Why need we save the QBName in both HintTable and TableOptimizerHint?
There was a problem hiding this comment.
Because we can specify the QB within which the hint will take effect, and also specify a table in some QB?
There was a problem hiding this comment.
For example: /*+ HASH_JOIN(@sel1 t1, t2) */ and /*+ HASH_JOIN(t1@sel1, t2@sel2).
There was a problem hiding this comment.
Yes, you can take a look at the test.
|
|
||
| TableOptimizerHintOpt: | ||
| index '(' HintTableAndIndexList ')' | ||
| index '(' QueryBlockOpt HintTable IndexNameList ')' |
There was a problem hiding this comment.
Do we have other way to do this? INDEX(@sel_1 t idx1, idx2, idx3) looks weird. Because IndexNameList here can not be empty, we can split by comma.
There was a problem hiding this comment.
It can be empty now.
| $$ = model.NewCIStr("") | ||
| } | ||
| | HintTableAndIndexList ',' Identifier | ||
| | singleAtIdentifier |
There was a problem hiding this comment.
I'm wondering whether we can use comma to split arguments in SQL hints too, it looks better.
| | singleAtIdentifier | |
| | singleAtIdentifier | |
| { | |
| $$ = model.NewCIStr($1) | |
| } | |
| | singleAtIdentifier ‘,’ |
There was a problem hiding this comment.
No, even if you want ,, it should not be put here.
| hintEnablePlanCache "ENABLE_PLAN_CACHE" | ||
| hintUsePlanCache "USE_PLAN_CACHE" | ||
| hintReadConsistentReplica "READ_CONSISTENT_REPLICA" | ||
| hintQBName "QB_NAME" |
There was a problem hiding this comment.
I'm not against using QBName in misc.go. But could we use the full name in parser.y to make its mean more clear?
There was a problem hiding this comment.
But full name would make using it harder beacause queryblock has 10 chars, it is reasonable trade off I think.
There was a problem hiding this comment.
I mean change hintQBName "QB_NAME" to hintQBName "QUERY_BLOCK_NAME".
There was a problem hiding this comment.
It is also what I means, It contains 8 + 1 more chars that QB_NAME.
foreyes
left a comment
foreyes
left a comment
There was a problem hiding this comment.
If we don't care a lot about args format, LGTM
|
@foreyes We can change it latter needed. Please approve it. |
3 participants
What problem does this PR solve?
Support query block syntax in hint.
What is changed and how it works?
Add optional query block as arugments for each hint.
Check List
Tests
Code changes
Side effects
Related changes