deniskuzZ commented on PR #6556:
URL: https://github.com/apache/hive/pull/6556#issuecomment-5508216186
Thanks for the patch. The diff is small, but it changes what the block below
runs on the normal execution path.
- explainConfig == null isn't the same as "not an explain". isExplainPlan and
explainConfig are independent Context fields, and ExplainSemanticAnalyzer
sets the config on runCtx without setExplainPlan(true). So with
hive.log.explain.output on, the EXPLAIN ANALYZE running phase now enters
here with the user's config, and for EXPLAIN ANALYZE CBO that runs
invalidateMetadataQuery()/THREAD_PROVIDERS.set() during the phase that
actually executes the query.
- setFormatted(true) selects JSON, but the logging path reaches getJSONPlan
with jsonOutput=false and does out.println(cboPlan) — one JSON blob under
"CBO PLAN:" in a text explain. RelOptUtil.toString() is what's readable in
a log; JSON is presumably needed only for the WEBUI_SHOW_GRAPH branch, so
the format wants picking per consumer.
- setCboJoinCost(true) never reaches the join-cost branch (isFormatted is
checked first); its only effect is skipping invalidateMetadataQuery().
- toJsonString() now runs inside the try at 572 caught at 676. Previously
only EXPLAIN reached it, where a failure just breaks the EXPLAIN; now it
can abort a real query, and at the default fallback strategy NEVER it's
rethrown rather than falling back. I'd guard the call.
- The condition only checks hive.log.explain.output, but explainOutput() also
fires on hive.server2.webui.explain.output and query-history explain.
More generally I'd rather keep plan serialization off the execution path —
this is troubleshooting instrumentation and can be requested directly via
EXPLAIN CBO / EXPLAIN FORMATTED. A separate flag defaulting to false, like
hive.log.explain.output.include.extended, would keep current behavior and
make the cost explicit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
