PHOENIX-7879 Tests for EXPLAIN text and ExplainPlanAttributes serialization compatibility - #2495
Conversation
…zation compatibility Co-authored-by: Claude <noreply@anthropic.com>
|
Force pushed a commit message addendum adding Claude as co-author. |
There was a problem hiding this comment.
Pull request overview
Adds a backward-compatibility “golden” test harness to freeze Phoenix EXPLAIN output (both plan-step text and ExplainPlanAttributes JSON) so future grammar changes become explicit and reviewable, while also stabilizing JSON serialization for previously problematic attributes.
Changes:
- Introduce
ExplainOracle+ExplainChangeRuleinfrastructure to compare normalized current EXPLAIN output against embedded baselines and produce readable diffs on mismatch. - Add text/JSON normalizers to strip environment-specific noise (parallelism counts, row/byte estimates, region locations, lookup counts, etc.).
- Stabilize
ExplainPlanAttributesJSON output ordering and add custom Jackson serializers forserverMergeColumnsandregionLocations.
Reviewed changes
Copilot reviewed 8 out of 8 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/ExplainTextNormalizer.java | Normalizes EXPLAIN text by eliding environment-specific details. |
| phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainJsonNormalizer.java | Normalizes EXPLAIN JSON attributes by nulling/zeroing cluster-dependent fields and recursing into nested plans. |
| phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainChangeRule.java | Provides rule hooks to transform “golden” expected text/JSON for intentional grammar changes. |
| phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainOracle.java | Orchestrates normalization, rule application, and mismatch diff generation for EXPLAIN comparisons. |
| phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainOracleTest.java | Corpus of compatibility tests plus serializer/order/normalizer sanity checks. |
| phoenix-core-client/src/main/java/org/apache/phoenix/compile/ServerMergeColumnsSerializer.java | Adds Jackson serialization for Set<PColumn> in plan attributes. |
| phoenix-core-client/src/main/java/org/apache/phoenix/compile/RegionLocationsListSerializer.java | Adds Jackson serialization for List<HRegionLocation> in plan attributes. |
| phoenix-core-client/src/main/java/org/apache/phoenix/compile/ExplainPlanAttributes.java | Freezes JSON property order and wires in the two custom serializers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…zation compatibility (#2495)
|
@apurtell @palashc did we have clean build results before merging the PR? The state of the build does not look good on master https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-mulitbranch/job/master/ |
|
PR builds are available with format https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-${PR_number} e.g. https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-2495 for this PR, since it is merged, it is no longer available to view. |
…zation compatibility (apache#2495)
…zation compatibility (apache#2495)
…zation compatibility (#2495)
…zation compatibility (#2495) Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com>
…zation compatibility (apache#2495) Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com>
…zation compatibility (#2495) Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com>
…zation compatibility (apache#2495) Co-authored-by: Claude Opus 4.8[1m] <noreply@anthropic.com>
Add a backward compatibility test harness that freezes Phoenix's
EXPLAINoutput and makes any future change to the grammar an explicit, reviewable act.ExplainPlanAttributesgains a@JsonPropertyOrderannotation, andRegionLocationsListSerializerandServerMergeColumnsSerializerhandle the two attributes that aren't cleanly serializable.New test package
org.apache.phoenix.query.explainimplements compatibility checks.ExplainOraclecompiles each query against a connectionless Phoenix driver and compares both the textual and JSON representations of the plan against an expected baseline, producing line-by-line and JSON-pointer diffs on mismatch. Normalizers strip environment-specific noise before comparison.ExplainTextNormalizercollapses parallelism/chunk counts to , removes row/byte stats and region-location lines, andExplainJsonNormalizernulls out region locations, lookup counts, split chunks, and row/size estimates and recurses into nested join plans.ExplainChangeRulesupports future PRs that intentionally change the grammar. New rules can be appended that transform the baseline into its new expected shape. Every change is intentional and reviewable. The diff in the rule itself documents exactly what changed.The golden text in
ExplainOracleTestshould be periodically regenerated with theExplainChangeRulesdropped afterward. So,ExplainChangeRuleis clearly optional. The golden text can simply be updated. However this could be a good practice.Generated-by: Claude Opus 4.8[1m] noreply@anthropic.com