[Multistage] Pushdown explain queries from controller to broker #10505
Conversation
tibrewalpratik17
changed the title
tibrewalpratik17
force-pushed
the
update_controller_pushdown
branch
from
28ba243 to
37d94b2
Compare
|
Thanks Pratik. cc: @walterddr |
Codecov Report
@@ Coverage Diff @@
## master #10505 +/- ##
============================================
- Coverage 70.21% 70.20% -0.01%
+ Complexity 6116 6115 -1
============================================
Files 2090 2090
Lines 112207 112210 +3
Branches 17078 17079 +1
============================================
- Hits 78784 78776 -8
- Misses 27857 27881 +24
+ Partials 5566 5553 -13
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 29 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| } else if (sqlNode instanceof SqlExplain) { | ||
| SqlExplain explain = (SqlExplain) sqlNode; | ||
| tableNames.addAll(extractTableNamesFromNode(explain.getExplicandum())); |
There was a problem hiding this comment.
This was never added previously. how come this become a problem after the broker tenant changes?
There was a problem hiding this comment.
This was a miss in the previous PR. In case of multistage queries via controller, we extract the table name from the query to see all the tables are in the same tenant or not. In case of explain plan for ... queries, we missed out on extracting name in the parser. Added it here in this patch.
There was a problem hiding this comment.
ah... i understand now! good catch!
This is a small follow-up to #10336. We weren't pushing down explain queries before. This PR allows to push explain plan queries in case of Multistage from controller to broker.
Thanks @ankitsultana for pointing this out!