PHOENIX-7881 Refactor UTs and ITs to assert on ExplainPlanAttributes#2497
Open
apurtell wants to merge 7 commits into
Open
PHOENIX-7881 Refactor UTs and ITs to assert on ExplainPlanAttributes#2497apurtell wants to merge 7 commits into
apurtell wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors a large set of unit/integration tests to assert EXPLAIN behavior via structured ExplainPlanAttributes (through a fluent assertPlan API) rather than brittle full-text EXPLAIN string matches. To support these assertions, it expands/propagates ExplainPlanAttributes in join/mutation plan implementations and improves plan normalizers to handle unstable dynamic-filter aliases.
Changes:
- Introduces
ExplainPlanTestUtilwith a fluentassertPlan/assertMutationPlanAPI for attribute-based EXPLAIN assertions. - Normalizes unstable
$N.$Ndynamic-filter aliases in both text and JSON explain normalizers, including nested subplans. - Extends structured explain attributes surfaced by core plans (e.g., HashJoinPlan subplans, SortMergeJoin skip-merge flag, group-by limit, delete single row).
Reviewed changes
Copilot reviewed 81 out of 81 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| phoenix-core/src/test/java/org/apache/phoenix/query/ExplainPlanTextTest.java | Removes legacy tests that asserted exact EXPLAIN text for DELETE plans. |
| phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainTextNormalizer.java | Normalizes dynamic-filter bind aliases in EXPLAIN text output. |
| phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTestUtil.java | Adds fluent helpers to get/verify ExplainPlanAttributes without relying on EXPLAIN text. |
| phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainJsonNormalizer.java | Normalizes dynamic-filter aliases and now recurses into subplans for JSON attributes. |
| phoenix-core/src/test/java/org/apache/phoenix/compile/StatementHintsCompilationTest.java | Migrates EXPLAIN assertions to assertPlan(...).iteratorType(...) etc. |
| phoenix-core/src/test/java/org/apache/phoenix/compile/QueryOptimizerTest.java | Migrates EXPLAIN assertions to structured scanType/table/keyRanges. |
| phoenix-core/src/test/java/org/apache/phoenix/compile/JoinQueryCompilerTest.java | Migrates join EXPLAIN assertions to structured attributes + subplan navigation. |
| phoenix-core/src/it/java/org/apache/phoenix/schema/ConditionalTTLExpressionIT.java | Replaces index-usage EXPLAIN checks with tableContains(...). |
| phoenix-core/src/it/java/org/apache/phoenix/rpc/PhoenixServerRpcIT.java | Refactors EXPLAIN plan checks to attribute assertions for index range scans. |
| phoenix-core/src/it/java/org/apache/phoenix/rpc/PhoenixClientRpcIT.java | Same as server RPC IT: uses structured explain assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewMetadataIT.java | Avoids brittle full-plan match by checking plan steps for index name presence. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertSelectWithRegionMovesIT.java | Migrates mutation EXPLAIN assertions to assertMutationPlan(...). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertSelectIT.java | Migrates mutation EXPLAIN assertions to assertMutationPlan(...). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDMLIT.java | Converts point-lookup explain checks to scanType(...) assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java | Converts explain checks to attribute assertions (scan type/table). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/TableTTLIT.java | Converts index/data-table explain checks to attribute assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/SubBinaryFunctionIT.java | Converts EXPLAIN substring checks to structured scan/key-range assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/SortMergeJoinMoreIT.java | Rewrites large plan-string comparisons into structured attribute assertions (incl. rhs()). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/ServerPagingIT.java | Replaces helper-based EXPLAIN checks with assertPlan(...).tableContains(...). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/salted/SaltedTableIT.java | Converts salted-table EXPLAIN checks to attribute assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/salted/BaseSaltedTableIT.java | Converts ORDER BY optimization EXPLAIN checks to attribute assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/RTrimFunctionIT.java | Converts EXPLAIN substring check to structured scan/table assertion. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorOffsetIT.java | Switches from text “With RVC Offset” checks to asserting hexStringRVCOffset. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryWithTableSampleIT.java | Migrates sampling explain checks to samplingRate/scanType/... attributes and join/subplan assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/ProjectArrayElemAfterHashJoinIT.java | Uses ExplainPlanAttributes directly and asserts array projection/subplan counts structurally. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/OnDuplicateKeyIT.java | Replaces plan-string validation with attribute-based index-scan assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackIT.java | Refactors explain-plan helper to attribute checks and normalized table naming. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/MaxLookbackExtendedIT.java | Same as MaxLookbackIT: attribute-based range-scan assertion. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/LogicalTableNameIT.java | Migrates EXPLAIN checks to attribute assertions; removes legacy plan-translation plumbing usage. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/LogicalTableNameExtendedIT.java | Migrates index hint EXPLAIN checks to tableContains. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/json/JsonFunctionsIT.java | Converts multiple explain substring checks to structured serverArrayElementProjection(...) etc. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SubqueryUsingSortMergeJoinIT.java | Replaces regex plan matching with structured join/subplan assertions; simplifies parameterization inputs. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SortMergeJoinNoSpoolingIT.java | Updates constructor signature to match new BaseJoinIT/SortMergeJoinIT parameterization. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SortMergeJoinNoIndexIT.java | Implements attribute-based assertions via overrides for plan-shape checks. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SortMergeJoinLocalIndexIT.java | Implements attribute-based assertions for local-index join plan shapes. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SortMergeJoinIT.java | Replaces plan-string equality checks with abstract hooks for attribute-based assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/join/SortMergeJoinGlobalIndexIT.java | Implements attribute-based assertions for global-index join plan shapes. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/join/HashJoinMoreIT.java | Converts hardcoded plan strings into structured join/subplan/dynamic-filter assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/join/BaseJoinIT.java | Removes legacy “virtual plan” translation + string-based plan match helpers. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolIT.java | Refactors explain assertions to use ExplainPlanAttributes (including case-sensitive/namespace name handling). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexBuildTimestampIT.java | Refactors explain assertions to attribute-based index scan checks. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ViewIndexIT.java | Migrates view-index explain assertions to structured attributes. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/UncoveredGlobalIndexRegionScannerIT.java | Converts multiple EXPLAIN checks to attribute assertions (scan type/table). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/UncoveredGlobalIndexRegionScanner2IT.java | Same as above: attribute-based EXPLAIN assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/txn/TxWriteFailureIT.java | Migrates EXPLAIN checks to structured scanType/table assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/SingleCellIndexIT.java | Replaces explain substring checks with assertPlan(...).table(...) assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/SaltedIndexIT.java | Converts multiple conditional expected-plan strings to structured assertions (iterator/limits/sorts). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PartialSystemCatalogIndexIT.java | Removes local EXPLAIN helper and asserts system catalog index scans via assertPlan. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PartialIndexIT.java | Uses ExplainPlanTestUtil.assertPlan for point-lookup/index usage cases. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexIT.java | Migrates explain checks for local/global index scans to structured assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexFailureIT.java | Replaces brittle substring matching with structured scan type + tableContains assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexUsageIT.java | Converts view/index usage explain strings to structured scan + serverWhereFilter assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexMaintenanceIT.java | Migrates EXPLAIN assertions to assertPlan(PhoenixPreparedStatement) for bound params. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/GlobalIndexOptimizationIT.java | Converts multiple expected-plan string comparisons to structured assertions, including regex-based dynamic-filter validation where needed. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/GlobalIndexCheckerWithRegionMovesIT.java | Refactors explain checks to structured attributes with normalized table-name comparison. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/GlobalIndexCheckerIT.java | Same as above, plus structured checks for server filters/distinct filters. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/BaseIndexWithRegionMovesIT.java | Converts uncovered-index explain-plan assertions to structured scan/merge/filter attributes. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/BaseIndexIT.java | Same as above (non-region-move base): structured assertions for scan/merge/filter. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/index/BaseImmutableIndexIT.java | Migrates immutable-index EXPLAIN checks to structured index scan assertions. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/ExplainPlanWithStatsDisabledIT.java | Refactors explain checks to use attributes while still validating region-location lookup counts. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/EmptyColumnIT.java | Converts distinct-prefix-filter explain checks to structured attributes (serverWhereFilter, distinct filter presence). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/DistinctPrefixFilterIT.java | Uses serverDistinctFilter presence/absence instead of string contains. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/DeleteIT.java | Refactors delete/index explain checks to structured attributes for both queries and mutations. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/CsvBulkLoadToolIT.java | Converts explain substring checks to tableContains(...). |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/ClientHashAggregateIT.java | Refactors explain checks to use client-aggregate/sorted-by attribute presence. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/CDCQueryIT.java | Ensures CDC index not used by checking scanned table name from structured attributes. |
| phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterSessionIT.java | Converts consistency explain checks to consistency(...) attribute assertions. |
| phoenix-core-client/src/main/java/org/apache/phoenix/execute/SortMergeJoinPlan.java | Populates sortMergeSkipMerge structured attribute consistently with textual “(SKIP MERGE)”. |
| phoenix-core-client/src/main/java/org/apache/phoenix/execute/HashJoinPlan.java | Adds structured attribute support for hash join subplans, dynamic filters, after-join filters, and join scanner limits. |
| phoenix-core-client/src/main/java/org/apache/phoenix/compile/GroupByCompiler.java | Populates serverGroupByLimit in ExplainPlanAttributes when applicable. |
| phoenix-core-client/src/main/java/org/apache/phoenix/compile/DeleteCompiler.java | Adds structured attributes to the “DELETE SINGLE ROW” explain plan. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactors ~70 UTs and ITs to assert on
ExplainPlanAttributesvia a fluentassertPlanAPI instead of hardcoding exactEXPLAINtext matches. ExpandsExplainPlanAttributeswith the additional fields the assertion API needs and reorganizes its attribute display order for better logical grouping.No changes to the actual
EXPLAINtext output format. Coverage is preserved. Some exact text matching is retained inQueryPlanTestso that coverage is not lost. Further refactoring toward fullassertPlanreliance is deferred until the newEXPLAINlogic and output-grammar validation land.Generated-by: Claude Opus 4.8[1m] noreply@anthropic.com