Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 63.79%. Comparing base (59551e4) to head (7c2a82a).
Report is 1162 commits behind head on master.

@@             Coverage Diff              @@
##             master   #14193      +/-   ##
============================================
+ Coverage     61.75%   63.79%   +2.04%     
- Complexity      207     1535    +1328     
============================================
  Files          2436     2622     +186     
  Lines        133233   144374   +11141     
  Branches      20636    22093    +1457     
============================================
+ Hits          82274    92108    +9834     
- Misses        44911    45472     +561     
- Partials       6048     6794     +746     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I was thinking about this feature and I think it may be better to have something like an explainMode option instead of having a boolean option. The main advantages are:

1. If in the future we add other modes, we can use the same property. We could even think about implementing EXPLAIN IMPLEMENTATION PLAN as a normal EXPLAIN PLAN with a different option.
2. We could use that property to unify all query engines. For example, we could modify single-stage query engine to return this kind of plan if the option is provided. Given we are working on a timeseries engine now, there may be even more cases like that in the future

I think that might be a good option given the proliferation of the different kinds of explain plans in Pinot. But we should probably discuss and come to a consensus on the best way forward here since there will be a lot more combinations possible with query options - i.e., different explainMode with EXPLAIN PLAN, EXPLAIN IMPLEMENTATION PLAN, EXPLAIN PLAN WITH IMPLEMENTATION, EXPLAIN PLAN WITHOUT IMPLEMENTATION. This can end up being super confusing for users if we aren't careful here - I'd suggest dropping support for at least EXPLAIN IMPLEMENTATION PLAN and putting that plan under something like explainMode=workers instead (assuming we decide upon that user facing terminology here). And maybe we can call out explicitly that explainMode can only be used with EXPLAIN PLAN - EXPLAIN PLAN WITHOUT IMPLEMENTATION will always return the basic logical plan without segment level info. Although things will still be a little tricky since EXPLAIN PLAN and EXPLAIN PLAN WITH IMPLEMENTATION are equivalent - what do we do when explainMode asks for logical plan without segment level info but is used with EXPLAIN PLAN WITH IMPLEMENTATION?

My suggestion is to modify the parser in order to support our different modes. It would be something like:

• EXPLAIN PLAN SEGMENT [INFO] FOR always shows the plan asking segments.
• EXPLAIN PLAN WITH LOGICAL [INFO] FOR always shows the logical plan.
• EXPLAIN PLAN WITH WORKER [INFO] FOR always shows the plan with workers.
• EXPLAIN PLAN FOR shows the plan asking segments or the logical plan depending on the configuration.

Or some other syntax like EXPLAIN SEGMENT PLAN, EXPLAIN WORKERS PLAN, EXPLAIN LOGICAL PLAN, etc. The current EXPLAIN PLAN WITH/WITHOUT IMPLEMENTATION and EXPLAIN IMPLEMENTATION PLAN should be still supported but we can either not document them or recommend to not use them