Uh oh!
There was an error while loading. Please reload this page.
PHOENIX-7917 Expand the EXPLAIN WITH options list grammar - #2525
Conversation
Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR expands Phoenix SQL’s EXPLAIN grammar from the legacy EXPLAIN [WITH REGIONS] <stmt> form to an option-list form EXPLAIN [(<opt> [, <opt>]*)] <stmt>, introducing a new ExplainOptions carrier object that is propagated through parsing/compilation and into StatementContext so explain rendering can be option-aware (while keeping EXPLAIN WITH REGIONS as a backward-compatible alias).
Changes:
- Extend
PhoenixSQL.gto parseEXPLAIN (...)option lists (plusEXPLAIN WITH REGIONSalias) and validate options into anExplainOptionsinstance. - Replace the old
ExplainTypeenum withExplainOptions, propagate it throughParseNodeFactory,ExplainStatement,PhoenixStatement, andStatementContext. - Gate region-locations emission in
ExplainTableon theREGIONSoption and update tests/ITs to request region locations explicitly where needed.
Reviewed changes
Copilot reviewed 14 out of 14 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 helpers to run optimization with explicit ExplainOptions and a convenience assertion entrypoint for REGIONS. |
| phoenix-core/src/test/java/org/apache/phoenix/parse/ExplainOptionsParserTest.java | Adds parser-level tests for the new EXPLAIN (...) option-list grammar and the legacy WITH REGIONS alias. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryLoggerIT.java | Updates EXPLAIN usage to align explain output comparisons with logged plans. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/BaseIndexIT.java | Switches region-location assertions to use the new “with regions” explain helper. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/FlappingLocalIndexIT.java | Ensures tests requesting region-location attributes enable REGIONS explicitly; adds test categorization annotation. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseAggregateIT.java | Switches region-location assertions to use the new “with regions” explain helper. |
| phoenix-core-client/src/main/java/org/apache/phoenix/parse/ParseNodeFactory.java | Updates explain(...) factory method to accept ExplainOptions. |
| phoenix-core-client/src/main/java/org/apache/phoenix/parse/ExplainType.java | Removes legacy ExplainType enum. |
| phoenix-core-client/src/main/java/org/apache/phoenix/parse/ExplainStatement.java | Stores parsed ExplainOptions (defaulting to ExplainOptions.DEFAULT). |
| phoenix-core-client/src/main/java/org/apache/phoenix/parse/ExplainOptions.java | Introduces ExplainOptions (regions/verbose/format) and a builder used by the parser. |
| phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java | Propagates ExplainOptions into compiled plan context so explain rendering can respect options. |
| phoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.java | Emits region-location text/attributes only when REGIONS is requested via ExplainOptions. |
| phoenix-core-client/src/main/java/org/apache/phoenix/compile/StatementContext.java | Adds ExplainOptions storage (defaulting to DEFAULT) with getters/setters. |
| phoenix-core-client/src/main/antlr3/PhoenixSQL.g | Extends grammar to parse option lists and validates allowed options into an ExplainOptions.Builder. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Test ResultsUnit tests ( |
| Test class | Tests | Failures | Errors | Skipped | Result |
|---|---|---|---|---|---|
ExplainOptionsParserTest (new) | 16 | 0 | 0 | 0 | PASS |
QueryParserTest | 93 | 0 | 0 | 0 | PASS |
ExplainPlanTest | 94 | 0 | 0 | 0 | PASS |
| Total | 203 | 0 | 0 | 0 | PASS |
ExplainOptionsParserTest covers the new grammar: REGIONS, VERBOSE, FORMAT TEXT, FORMAT JSON, case-insensitivity, option ordering, the legacy WITH REGIONS alias, and the error paths (unknown option, unknown format kind, duplicate option, FORMAT without a value, REGIONS with a value, and mixing the legacy alias with the option list).
Integration tests (*IT)
| Test class | Category | Tests | Failures | Errors | Skipped | Result |
|---|---|---|---|---|---|---|
AggregateIT | ParallelStatsDisabled | 25 | 0 | 0 | 0 | PASS |
GlobalImmutableNonTxIndexIT | ParallelStatsDisabled | 46 | 0 | 0 | 0 | PASS |
LocalMutableNonTxIndexIT | ParallelStatsDisabled | 46 | 0 | 0 | 4 | PASS |
QueryLoggerIT | NeedsOwnMiniCluster | 7 | 0 | 0 | 0 | PASS |
FlappingLocalIndexIT | NeedsOwnMiniCluster | 12 | 0 | 0 | 0 | PASS |
AggregateITdrives the three migrated sites inBaseAggregateIT(testGroupByOrderPreserving,testSumGroupByOrderPreserving,testAvgGroupByOrderPreserving).GlobalImmutableNonTxIndexIT/LocalMutableNonTxIndexITdrive the three migrated sites inBaseIndexIT(testIndexWithNullableFixedWithCols,testIndexWithNullableDateCol,testSelectAllAndAliasWithIndex). The 4 skipped tests inLocalMutableNonTxIndexITare pre-existing failing tests unrelated to this change tracked by PHOENIX-7893.FlappingLocalIndexITwas previously uncategorized and therefore skipped by all group-filtered failsafe executions. It is now annotated@Category(NeedsOwnMiniClusterTest.class).
Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Replace the
EXPLAIN [WITH REGIONS]grammar with a comma-separated option listEXPLAIN [(<opt> [, <opt>]*)] <stmt>acceptingREGIONS,VERBOSE,FORMAT TEXT, andFORMAT JSON, preservingEXPLAIN WITH REGIONSas a backward compatible alias.Update
PhoenixSQL.gexplain_noderule to accept eitherLPAREN id (COMMA id)* RPARENorWITH REGIONSbetweenEXPLAINand the inneroneStatement, withFORMAT TEXT|JSONparsed asFORMATfollowed by an identifier.VERBOSE,FORMAT,TEXT, andJSONare matched asidentifierinsideexplain_nodeand validated against a closed set in the action block.ExplainStatementreplacesExplainTypewithExplainOptions { boolean regions; boolean verbose; Format format; }.ParseNodeFactorygains afactory.explain(stmt, ExplainOptions)overload.PhoenixStatement#ExecutableExplainStatementtakesExplainOptionsand propagatesregionsthrough toBaseResultIterators.explainUtil.Region locator lookup still happens unconditionally.
Co-authored-by: Claude Opus 4.8[1m] noreply@anthropic.com