Skip to content

PHOENIX-7881 Refactor UTs and ITs to assert on ExplainPlanAttributes - #2503

Merged
apurtell merged 1 commit into
apache:PHOENIX-7876-featurefrom
apurtell:PHOENIX-7881
Jun 9, 2026
Merged

PHOENIX-7881 Refactor UTs and ITs to assert on ExplainPlanAttributes#2503
apurtell merged 1 commit into
apache:PHOENIX-7876-featurefrom
apurtell:PHOENIX-7881

Conversation

@apurtell

@apurtellapurtell commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Refactor every IT and UT that asserted on EXPLAIN text to use the new fluent ExplainPlanTestUtil.assertPlan API over ExplainPlanAttributes.

Emitted text and JSON shape are unchanged. Coverage is preserved.

ExplainPlanAttributes is regrouped and gains an ordered clientSteps list plus lhsJoinQueryExplainPlan, subPlans, dynamicServerFilter, afterJoinFilter, joinScannerLimit, sortMergeSkipMerge, serverGroupByLimit, regionLocationsTotalSize, and numRegionLocationLookups. SortMergeJoinPlan.getExplainPlan() is reshaped into a synthetic root carrying the join operator with separate lhs and rhs children. HashJoinPlan.getExplainPlan() now populates attributes for sub-plans, post join filters, and scanner limits. ClientAggregatePlan and ClientScanPlan plus the merge-sort, ordered, limiting, and offset iterators add clientSteps to plan explanation. ExplainPlanTestUtil adds lhs() and rhs() navigation and various new assertions such as clientStepCount, clientStep(i), clientSteps(...), and others. ExplainJsonNormalizer recurses into lhs, rhs, and subPlans.

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

apurtell
apurtell commented Jun 9, 2026
View reviewed changes
Comment threadphoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.java
return (useLongViewIndex ? (Long) s : (Short) s) + Short.MAX_VALUE + 2;
}

private static class RowKeyValueIterator implements Iterator<byte[]> {

@apurtellapurtellJun 9, 2026•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is me removing some unused code I found.

@apurtell
apurtell requested a review from CopilotJune 9, 2026 00:11
CopilotAI reviewed Jun 9, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors Phoenix unit/integration tests to stop asserting against raw EXPLAIN text and instead assert against structured ExplainPlanAttributes via the fluent ExplainPlanTestUtil.assertPlan(...) API, while extending explain-plan attribute capture (e.g., client-side steps, join operands, sub-plans) to keep emitted text/JSON stable.

Changes:

  • Migrated many UTs/ITs from string-based EXPLAIN assertions (QueryUtil.getExplainPlan(...)) to fluent attribute-based assertions.
  • Extended explain-plan attribute plumbing to capture ordered clientSteps, richer join structure (lhs/rhs), and additional join/subplan-related attributes.
  • Updated explain normalizers (text/JSON) to accommodate new attribute shape and recursion paths.

Reviewed changes

Copilot reviewed 143 out of 143 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
phoenix-core/src/test/java/org/apache/phoenix/query/ExplainPlanTextTest.javaRemoved legacy text-only EXPLAIN test
phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainTextNormalizer.javaClarified normalizer comments
phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainJsonNormalizer.javaNormalize new explain JSON fields/children
phoenix-core/src/test/java/org/apache/phoenix/compile/StatementHintsCompilationTest.javaUse assertPlan for hint explain assertions
phoenix-core/src/test/java/org/apache/phoenix/compile/QueryOptimizerTest.javaUse assertPlan for optimizer explain assertions
phoenix-core/src/test/java/org/apache/phoenix/compile/JoinQueryCompilerTest.javaUse assertPlan + subPlan assertions
phoenix-core/src/it/java/org/apache/phoenix/schema/ConditionalTTLExpressionIT.javaUse assertPlan for index usage checks
phoenix-core/src/it/java/org/apache/phoenix/rpc/PhoenixServerRpcIT.javaUse assertPlan on prepared statements
phoenix-core/src/it/java/org/apache/phoenix/rpc/PhoenixClientRpcIT.javaUse assertPlan on prepared statements
phoenix-core/src/it/java/org/apache/phoenix/monitoring/HBaseScanMetricsIT.javaUse assertPlan for scanned table
phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyNullablePKIT.javaUse assertPlan for index usage
phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyITBase.javaUse assertPlan for scan-type checks
phoenix-core/src/it/java/org/apache/phoenix/end2end/WhereOptimizerForArrayAnyIT.javaUse assertPlan for scan-type checks
phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewMetadataIT.javaUse assertPlan/getPlanSteps for index mention
phoenix-core/src/it/java/org/apache/phoenix/end2end/VarBinaryEncoded2IT.javaUse assertPlan for index + scan type
phoenix-core/src/it/java/org/apache/phoenix/end2end/UserDefinedFunctionsIT.javaUse assertPlan for functional index plans
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertSelectWithRegionMovesIT.javaUse assertMutationPlan for mutation explain
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertSelectIT.javaUse assertMutationPlan for mutation explain
phoenix-core/src/it/java/org/apache/phoenix/end2end/UnionAllIT.javaAssert union root + clientSteps + rhs
phoenix-core/src/it/java/org/apache/phoenix/end2end/UCFWithDisabledIndexIT.javaUse assertPlan for scanned table
phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificViewIndexIT.javaUse assertPlan for range scan plan
phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDMLIT.javaUse assertPlan for point-lookup scan type
phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.javaUse assertPlan for hinted index usage
phoenix-core/src/it/java/org/apache/phoenix/end2end/TableTTLIT.javaUse assertPlan for index vs data usage
phoenix-core/src/it/java/org/apache/phoenix/end2end/SubBinaryFunctionIT.javaUse assertPlan for range-scan key ranges
phoenix-core/src/it/java/org/apache/phoenix/end2end/SpillableGroupByIT.javaUse assertPlan for splits/scan/table
phoenix-core/src/it/java/org/apache/phoenix/end2end/SortMergeJoinMoreIT.javaUse assertPlan with lhs()/rhs() navigation
phoenix-core/src/it/java/org/apache/phoenix/end2end/ServerPagingIT.javaUse assertPlan for index table selection
phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceIT.javaUse assertPlan for client sequence count
phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceBulkAllocationIT.javaUse assertPlan for sequence explain assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/salted/SaltedTableIT.javaUse assertPlan for salted point lookups
phoenix-core/src/it/java/org/apache/phoenix/end2end/salted/BaseSaltedTableIT.javaUse assertPlan for order-by optimization
phoenix-core/src/it/java/org/apache/phoenix/end2end/RTrimFunctionIT.javaUse assertPlan for range scan
phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorOffsetIT.javaUse assertPlan for offset explain attributes
phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorIT.javaUse assertPlan for skip-scan plan assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/ReverseScanIT.javaUse assertPlan for reverse scan assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryWithTableSampleIT.javaUse assertPlan for sampling-rate explains
phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryWithOffsetIT.javaUse assertPlan for offset plan assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryWithLimitIT.javaUse assertPlan for limit plan assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryLoggerIT.javaAdded comments re: logged EXPLAIN_PLAN
phoenix-core/src/it/java/org/apache/phoenix/end2end/ProjectArrayElemAfterHashJoinIT.javaUse attributes + assertPlan for projection/subplans
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParallelStatsDisabledWithRegionMovesIT.javaRemoved legacy validateQueryPlan helper
phoenix-core/src/it/java/org/apache/phoenix/end2end/ParallelStatsDisabledIT.javaRemoved legacy validateQueryPlan helper
phoenix-core/src/it/java/org/apache/phoenix/end2end/OnDuplicateKeyIT.javaUse assertPlan + normalized index naming
phoenix-core/src/it/java/org/apache/phoenix/end2end/OnDuplicateKey2IT.javaUse assertPlan for scanned table name
phoenix-core/src/it/java/org/apache/phoenix/end2end/MultiCfQueryExecIT.javaUse assertPlan for FORWARD_SCAN sorting
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.javaUse assertPlan for index scan verification
phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackExtendedIT.javaUse assertPlan for index scan verification
phoenix-core/src/it/java/org/apache/phoenix/end2end/LogicalTableNameIT.javaUse assertPlan; simplify join test wiring
phoenix-core/src/it/java/org/apache/phoenix/end2end/LogicalTableNameExtendedIT.javaUse assertPlan; derive hinted index
phoenix-core/src/it/java/org/apache/phoenix/end2end/LocalIndexSplitMergeIT.javaUse assertPlan for local index plans
phoenix-core/src/it/java/org/apache/phoenix/end2end/KeyOnlyIT.javaUse assertPlan for limit plan assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/json/JsonFunctionsIT.javaUse assertPlan for index/table/projection checks
phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SortMergeJoinNoSpoolingIT.javaConstructor signature aligned with base
phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SortMergeJoinNoIndexIT.javaProvide attribute-based join plan assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SortMergeJoinIT.javaReplace string plans with assertion hooks
phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SortMergeJoinGlobalIndexIT.javaAttribute-based assertions for global-index cases
phoenix-core/src/it/java/org/apache/phoenix/end2end/join/BaseJoinIT.javaRemoved string-plan translation/assertion helpers
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolForPartialBuildIT.javaUse assertPlan for data vs index scans
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexExtendedIT.javaUse assertPlan for mutable index checks
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexBuildTimestampIT.javaUse assertPlan for local/global index scans
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ViewIndexIT.javaUse assertPlan for view-index plans
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/UncoveredGlobalIndexRegionScannerIT.javaUse assertPlan for scan/table selection
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/UncoveredGlobalIndexRegionScanner2IT.javaUse assertPlan for scan/table selection
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/txn/TxWriteFailureIT.javaUse assertPlan for data/index scan asserts
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/SingleCellIndexIT.javaUse assertPlan for index usage asserts
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PartialSystemCatalogIndexIT.javaUse assertPlan for SYSTEM.CATALOG explain
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexFailureIT.javaUse assertPlan for failure/retry plan checks
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexMaintenanceIT.javaUse assertPlan for local/global index scans
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ImmutableIndexWithStatsIT.javaUse assertPlan for stats-related scan type
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ChildViewsUseParentViewIndexIT.javaUse assertPlan for view-index vs base-table
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/BaseImmutableIndexIT.javaUse assertPlan for global immutable index scans
phoenix-core/src/it/java/org/apache/phoenix/end2end/EmptyColumnIT.javaUse attributes to assert distinct prefix filter
phoenix-core/src/it/java/org/apache/phoenix/end2end/DistinctPrefixFilterIT.javaUse attributes to assert distinct prefix filter
phoenix-core/src/it/java/org/apache/phoenix/end2end/DeleteIT.javaUse assertPlan/assertMutationPlan for delete explain
phoenix-core/src/it/java/org/apache/phoenix/end2end/CsvBulkLoadToolIT.javaUse assertPlan for index usage asserts
phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.javaUse assertPlan for namespace-mapped table scans
phoenix-core/src/it/java/org/apache/phoenix/end2end/CountDistinctApproximateHyperLogLogIT.javaUse assertPlan for aggregate scan assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/ClientHashAggregateIT.javaUse attributes to assert client hash aggregate
phoenix-core/src/it/java/org/apache/phoenix/end2end/CDCQueryIT.javaUse attributes to assert CDC index not used
phoenix-core/src/it/java/org/apache/phoenix/end2end/CastAndCoerceIT.javaUse assertPlan for range scan assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/Bson5IT.javaUse assertPlan; add rawtype suppressions
phoenix-core/src/it/java/org/apache/phoenix/end2end/Bson4IT.javaUse assertPlan for scan/table assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.javaUse assertPlan for view index plan assertions
phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseTenantSpecificViewIndexIT.javaUse assertPlan for tenant view index plans
phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.javaUse assertPlan/attributes for index usage checks
phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseOrderByWithRegionMovesIT.javaUse assertPlan for order-by optimization asserts
phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseOrderByIT.javaUse assertPlan for order-by optimization asserts
phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseAggregateWithRegionMovesIT.javaUse assertPlan for aggregate plan asserts
phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterSessionIT.javaUse assertPlan for consistency explain asserts
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/SequenceResultIterator.javaAdd clientSteps recording for sequence reserve
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/SegmentResultIterator.javaAdd clientSteps recording for segment scan
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/OrderedResultIterator.javaAdd clientSteps recording for ordered iterator
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/OffsetResultIterator.javaAdd clientSteps recording for offset iterator
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/MergeSortTopNResultIterator.javaAdd clientSteps recording for merge sort/limit
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/MergeSortRowKeyResultIterator.javaAdd clientSteps recording for merge sort
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/LimitingResultIterator.javaAdd clientSteps recording for row limit
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/FilterResultIterator.javaAdd clientSteps recording for client filter
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/FilterAggregatingResultIterator.javaAdd clientSteps recording for client filter
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/DistinctAggregatingResultIterator.javaAdd clientSteps recording for distinct
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/CursorResultIterator.javaAdd clientSteps recording for cursor
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.javaAdd regionLocationsTotalSize in attributes
phoenix-core-client/src/main/java/org/apache/phoenix/execute/TupleProjectionPlan.javaAdd clientSteps recording for post-filter
phoenix-core-client/src/main/java/org/apache/phoenix/execute/SortMergeJoinPlan.javaReshape explain attributes to synthetic join root
phoenix-core-client/src/main/java/org/apache/phoenix/execute/HashJoinPlan.javaPopulate attributes for subPlans/filters/limits
phoenix-core-client/src/main/java/org/apache/phoenix/execute/ClientScanPlan.javaRecord client steps in attributes
phoenix-core-client/src/main/java/org/apache/phoenix/execute/ClientAggregatePlan.javaRecord client steps in attributes
phoenix-core-client/src/main/java/org/apache/phoenix/compile/GroupByCompiler.javaPopulate serverGroupByLimit in attributes
phoenix-core-client/src/main/java/org/apache/phoenix/compile/DeleteCompiler.javaProvide attributes for DELETE SINGLE ROW explain

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

Comment threadphoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainJsonNormalizer.java
@apurtell

apurtell commented Jun 9, 2026•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Updated with an accepted improvement suggested by Copilot and some test fixes. The issues were test issues introduced by the changes and do not change Phoenix query planning or function:

  • serverRowLimit attribute in ExplainPlanAttributes was not always populated
  • Test assertions against the synthetic Sort-Merge-Join root used the wrong side
  • Nested left hand side navigation of Sort-Merge-Join trees by test assertions was not correct
  • CastAndCoerceIT re-prepared a query, dropping bindings, confusing assertPlan

@apurtell

apurtell commented Jun 9, 2026

Copy link
Copy Markdown
ContributorAuthor

Test results

Scope

git show --stat 7c6609046d ("PHOENIX-7881 Refactor UTs and ITs to assert on ExplainPlanAttributes") touches 143 files (+4,819 / −5,241): 20 production (src/main), 112 integration (src/it), 11 unit (src/test).

The only added runtime behavior is attribute population. Text rendering is byte-for-byte unchanged.

Surefire (unit tests)

mvn -pl phoenix-core test -DforkCount=6 -Djacoco.skip=true \
-Dtest='ExplainPlanTest,JoinQueryCompilerTest,ParallelIteratorsSplitTest,PostIndexDDLCompilerTest,QueryCompilerTest,QueryOptimizerTest,QueryPlanTest,StatementHintsCompilationTest,TenantSpecificViewIndexCompileTest'
Test classRunFailuresErrorsSkipped
query.explain.ExplainPlanTest (text corpus)59000
compile.QueryCompilerTest215002
compile.QueryOptimizerTest49001
compile.TenantSpecificViewIndexCompileTest6000
compile.StatementHintsCompilationTest4000
query.QueryPlanTest4000
compile.PostIndexDDLCompilerTest3000
compile.JoinQueryCompilerTest2000
query.ParallelIteratorsSplitTest8000
Total350003

Failsafe (integration tests)

Full impacted-IT surface = 94 concrete classes (forkCount=2, reuseForks=false, -Djacoco.skip=true), run across three sessions. All EXPLAIN assertions pass.

PhaseClassesTests runPassFailError (env.)Skip
Surefire9350350003
Failsafe943,2153,18503093
All1033,5653,53503096

Zero EXPLAIN-assertion failures across the entire impacted surface.

@apurtell
apurtell merged commit ec1e291 into apache:PHOENIX-7876-featureJun 9, 2026
@apurtell
apurtell deleted the PHOENIX-7881 branch June 9, 2026 06:25
asf-gitbox-commits pushed a commit that referenced this pull request Jun 10, 2026
@apurtell
PHOENIX-7881Refactor UTs and ITs to assert on ExplainPlanAttributes (#…
5417883
…2503)
asf-gitbox-commits pushed a commit that referenced this pull request Jun 10, 2026
@apurtell@claude
PHOENIX-7881Refactor UTs and ITs to assert on ExplainPlanAttributes (#…
1cf776b
…2503)
Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com>
apurtell added a commit to apurtell/phoenix that referenced this pull request Jun 17, 2026
@apurtell@claude
PHOENIX-7881 Refactor UTs and ITs to assert on ExplainPlanAttributes (a…
f155e7e
…pache#2503)
Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com>
asf-gitbox-commits pushed a commit that referenced this pull request Jul 24, 2026
@apurtell@claudeAndrew Purtell
PHOENIX-7881Refactor UTs and ITs to assert on ExplainPlanAttributes (#…
dc5ef00
…2503)
Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com>
apurtell added a commit to apurtell/phoenix that referenced this pull request Aug 4, 2026
@apurtell@claudeAndrew Purtell
PHOENIX-7881 Refactor UTs and ITs to assert on ExplainPlanAttributes (a…
353197e
…pache#2503)
Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@apurtell