Rename multi-stage engine's explain asking servers to explain include segment plan

[yashmayya]

• https://github.com/apache/pinot/pull/13733
• The broker config was renamed from pinot.query.explain.ask.servers to pinot.query.multistage.explain.include.segment.plan during the course of review. This patch updates the corresponding query option to match the name for a consistent user experience (and also updates a couple of internal usages of the term).

[codecov-commenter]

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.

Files with missing lines	Patch %	Lines
...requesthandler/MultiStageBrokerRequestHandler.java	0.00%	1 Missing :warning:
...e/pinot/common/utils/config/QueryOptionsUtils.java	0.00%	1 Missing :warning:
.../java/org/apache/pinot/query/QueryEnvironment.java	0.00%	1 Missing :warning:

Additional details and impacted files

@@             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     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (+99.99%)	:arrow_up:
integration	100.00% <ø> (+99.99%)	:arrow_up:
integration1	100.00% <ø> (+99.99%)	:arrow_up:
integration2	0.00% <ø> (ø)
java-11	63.77% <0.00%> (+2.06%)	:arrow_up:
java-21	63.69% <0.00%> (+2.06%)	:arrow_up:
skip-bytebuffers-false	63.79% <0.00%> (+2.04%)	:arrow_up:
skip-bytebuffers-true	63.67% <0.00%> (+35.94%)	:arrow_up:
temurin	63.79% <0.00%> (+2.04%)	:arrow_up:
unittests	63.79% <0.00%> (+2.04%)	:arrow_up:
unittests1	55.49% <0.00%> (+8.60%)	:arrow_up:
unittests2	34.32% <0.00%> (+6.59%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[gortiz]

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

[yashmayya]

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?

[gortiz]

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