PHOENIX-7930: Address review feedback for EXPLAIN improvements

[apurtell]

Address review feedback for EXPLAIN improvements

• StatementContext.adoptRewriteState shares mutable collections by reference. Have adoptRewriteState and the copy constructor allocate fresh collections.
• In ExplainPlanAttributes, serverMergeColumns, subPlans, and regionLocations are the only fields not wrapped with Collections.unmodifiableX(...). Callers can mutate after build, silently corrupting EXPLAIN JSON. Wrap these three fields the same way the others are wrapped.
• Remove an unwanted catch of Exception in ExplainTable. Remove now unused SLF4J imports.
• PHOENIX-7928 changed 14 index rule assertions to a simple substring match on "matches" because the index type is a functional index. Restore and improve the affected assertions to specific matches for the test case.

Co-authored-by: Claude Opus 4.8[1m] noreply@anthropic.com

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR addresses review feedback around Phoenix EXPLAIN improvements by preventing shared mutable state during compilation, hardening ExplainPlanAttributes immutability, and tightening functional-index optimizer rule assertions in tests.

Changes:

• Deep-copy rewrite/diagnostic state in StatementContext (copy ctor + adoptRewriteState) to avoid shared mutable collections.
• Wrap remaining mutable ExplainPlanAttributes fields (serverMergeColumns, subPlans, regionLocations) with unmodifiable wrappers.
• Remove broad exception swallowing in ExplainTable and update tests/utilities to assert functional-index rule labels via OptimizerReasons.matches(...).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTestUtil.java	Adds indexRuleMatches() helper using OptimizerReasons.matches() for exact rule-label assertions.
phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTest.java	Updates functional-index rule assertion to use indexRuleMatches().
phoenix-core/src/it/java/org/apache/phoenix/end2end/json/JsonFunctionsIT.java	Tightens plan assertion to match exact functional-index rule label.
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexUsageIT.java	Restores/improves functional-index assertions using exact match helper across multiple ITs.
phoenix-core-client/src/main/java/org/apache/phoenix/optimize/QueryOptimizer.java	Removes a stray semicolon.
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.java	Removes catch (Exception) + logging and keeps region location formatting/attributes setting.
phoenix-core-client/src/main/java/org/apache/phoenix/compile/UnionCompiler.java	Records UNION ORDER BY MERGE breadcrumb on the real (outer) compilation context.
phoenix-core-client/src/main/java/org/apache/phoenix/compile/StatementContext.java	Implements deep(ish) copy of rewrite/diagnostic collections to avoid shared mutable state.
phoenix-core-client/src/main/java/org/apache/phoenix/compile/QueryCompiler.java	Passes the outer compilation context into UnionCompiler for breadcrumb recording.
phoenix-core-client/src/main/java/org/apache/phoenix/compile/ExplainPlanAttributes.java	Wraps previously-mutable collections in unmodifiable wrappers to prevent post-build mutation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.