Skip to content

PHOENIX-7921 Remove unread bookkeeping state from StatementContext - #2528

Merged
apurtell merged 1 commit into
apache:PHOENIX-7876-featurefrom
apurtell:PHOENIX-7921
Jun 14, 2026
Merged

PHOENIX-7921 Remove unread bookkeeping state from StatementContext#2528
apurtell merged 1 commit into
apache:PHOENIX-7876-featurefrom
apurtell:PHOENIX-7921

Conversation

@apurtell

Copy link
Copy Markdown
Contributor

Remove unused bookkeeping from StatementContext. Drops unionTags, addLiftedHavingNode(), isLiftedHavingNode() and the backing liftedHavingNodes set (the HAVING-lift breadcrumb is still emitted as a REWRITE HAVING PREDICATE AS WHERE line via appliedRewrites), and addIndexExpressionSubstitution() / getIndexExpressionSubstitutions() and the backing indexExpressionSubstitutions list. Removes the corresponding fields, their entries in the copy constructor, the no-arg-init constructor, and adoptRewriteState, plus the now-dead breadcrumbContext.addIndexExpressionSubstitution() call in IndexExpressionParseNodeRewriter and the addLiftedHavingNode loop in HavingCompiler. No behavior change. EXPLAIN text and JSON output are unchanged.

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

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

Removes unused diagnostic/bookkeeping state from StatementContext and deletes the now-dead call sites that populated it, while preserving the existing rewrite breadcrumbs (appliedRewrites) used for EXPLAIN output.

Changes:

  • Remove indexExpressionSubstitutions tracking and related APIs from StatementContext, plus the dead recorder call in IndexExpressionParseNodeRewriter.
  • Remove liftedHavingNodes / addLiftedHavingNode() / isLiftedHavingNode() from StatementContext, plus the dead recording loop in HavingCompiler.
  • Remove the unused unionTags() helper from StatementContext.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

FileDescription
phoenix-core-client/src/main/java/org/apache/phoenix/parse/IndexExpressionParseNodeRewriter.javaDrops the now-unused breadcrumbContext.addIndexExpressionSubstitution(...) call, leaving breadcrumb emission via addAppliedRewrite(...).
phoenix-core-client/src/main/java/org/apache/phoenix/compile/StatementContext.javaDeletes unused fields and methods (indexExpressionSubstitutions, liftedHavingNodes, unionTags) and removes their propagation via constructors/adoption.
phoenix-core-client/src/main/java/org/apache/phoenix/compile/HavingCompiler.javaRemoves the loop that recorded lifted HAVING parse nodes, retaining the rewrite breadcrumb.

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

@apurtell

Copy link
Copy Markdown
ContributorAuthor

Test Results

Unit tests (*Test) — full reactor

ModuleTestsFailuresErrorsSkippedResult
phoenix-core-client0000no unit tests in module
phoenix-core-server0000no unit tests in module
phoenix-core2580009PASS

All unit tests covering the modified code paths passed:

Test classTestsResult
query.explain.ExplainPlanTest105PASS
query.explain.ExplainJsonOutputTest2PASS
parse.ExplainOptionsParserTest16PASS
compile.HavingCompilerTest11PASS
compile.QueryOptimizerTest52PASS
compile.WhereOptimizerTest138PASS
compile.QueryCompilerTest215PASS

Impacted integration tests (*IT)

Selected because they directly assert EXPLAIN output through the modified compile paths (functional/partial-index expression rewrite, HAVING-as-WHERE lift, derived-table flatten).

IT classTestsFailuresErrorsSkippedTimeResult
end2end.ExplainPlanWithStatsDisabledIT1700073.9 sPASS
end2end.DerivedTableIT1800086.7 sPASS
end2end.ExplainPlanWithStatsEnabledIT30000101.6 sPASS
end2end.index.PartialIndexIT64000453.9 sPASS
Total129000BUILD SUCCESS

@apurtell
apurtell merged commit 66be274 into apache:PHOENIX-7876-featureJun 14, 2026
@apurtell
apurtell deleted the PHOENIX-7921 branch June 14, 2026 00:40
apurtell added a commit to apurtell/phoenix that referenced this pull request Jun 17, 2026
…pache#2528)
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
…2528)
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
…pache#2528)
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@apurtell