Skip to content

Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue#18153

Open
rsrkpatwari1234 wants to merge 10 commits intoapache:masterfrom
rsrkpatwari1234:rsrkpatwari1234-cast-issue
Open

Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue#18153
rsrkpatwari1234 wants to merge 10 commits intoapache:masterfrom
rsrkpatwari1234:rsrkpatwari1234-cast-issue

Conversation

@rsrkpatwari1234
Copy link
Copy Markdown
Contributor

Fixes #18152

After normalizing the cast target (BIGINT→LONG), recurse into operand 0 so nested CASTs and CASTs inside divide/etc. are fully rewritten.

bug backward-incompat dependencies multi-stage

@rsrkpatwari1234 rsrkpatwari1234 changed the title Allow CastTypeAliasRewriter to recurse into nested CAST operands to f… Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue Apr 9, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.14%. Comparing base (7e10a36) to head (4d2c294).

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18153      +/-   ##
============================================
- Coverage     63.18%   63.14%   -0.04%     
  Complexity     1616     1616              
============================================
  Files          3214     3214              
  Lines        195838   195840       +2     
  Branches      30251    30251              
============================================
- Hits         123734   123666      -68     
- Misses        62236    62296      +60     
- Partials       9868     9878      +10     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.10% <100.00%> (-0.03%) ⬇️
java-21 63.12% <100.00%> (-0.05%) ⬇️
temurin 63.14% <100.00%> (-0.04%) ⬇️
unittests 63.14% <100.00%> (-0.04%) ⬇️
unittests1 55.37% <100.00%> (-0.02%) ⬇️
unittests2 34.77% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Jackie-Jiang Jackie-Jiang added bug Something is not working as expected query Related to query processing labels Apr 9, 2026
@Jackie-Jiang Jackie-Jiang requested review from Copilot and yashmayya and removed request for yashmayya April 9, 2026 21:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a backward-compatibility bug in the SQL CAST type-alias rewriter by ensuring it rewrites nested CAST expressions inside the first operand of an outer CAST (e.g., CAST(CAST(x AS LONG) AS DOUBLE), or CAST((CAST(x AS LONG)/1000) AS DOUBLE)), which previously could break mixed-version clusters.

Changes:

  • Update CastTypeAliasRewriter to recurse into CAST operand 0 after normalizing the cast target type (e.g., BIGINT → LONG).
  • Add comprehensive unit tests covering nested CASTs in SELECT, WHERE, GROUP BY, and under arithmetic.
  • Extend the compatibility verifier sample suite with nested-CAST queries and expected results.

Reviewed changes

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

File Description
pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/CastTypeAliasRewriter.java Recurses into the CAST expression operand so nested CAST nodes get rewritten.
pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/CastTypeAliasRewriterTest.java Adds coverage for nested CAST rewrite behavior across multiple query locations/shapes.
compatibility-verifier/sample-test-suite/config/queries/feature-test-1-sql.queries Adds sample nested-CAST queries that previously could fail during rolling upgrades.
compatibility-verifier/sample-test-suite/config/query-results/feature-test-1-rest-sql.results Adds expected outputs for the new nested-CAST compatibility queries.

@rsrkpatwari1234
Copy link
Copy Markdown
Contributor Author

@rohityadav1993 @Jackie-Jiang @yashmayya Requesting your review on this

Copy link
Copy Markdown
Collaborator

@shauryachats shauryachats left a comment

Choose a reason for hiding this comment

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

LGTM

@yashmayya
Copy link
Copy Markdown
Contributor

@rsrkpatwari1234 the release candidate for 1.5.0 has already been built -- this patch is only needed for upgrade compatibility right? Is it even useful once servers are able to process BIGINT directly?

During rolling upgrades (Controller+Broker -> 1.4 and Server -> 1.3), queries that nest CAST(... AS LONG) (→ BIGINT in the plan) under another CAST or under an expression that is the first operand of an outer CAST can fail on older servers because CastTypeAliasRewriter never visited those inner CASTs before dispatch.

I'm not sure I follow this - CastTypeAliasRewriter doesn't even exist in 1.4 (https://github.com/apache/pinot/commits/release-1.4.0/).

@rsrkpatwari1234
Copy link
Copy Markdown
Contributor Author

@rsrkpatwari1234 the release candidate for 1.5.0 has already been built -- this patch is only needed for upgrade compatibility right? Is it even useful once servers are able to process BIGINT directly?

During rolling upgrades (Controller+Broker -> 1.4 and Server -> 1.3), queries that nest CAST(... AS LONG) (→ BIGINT in the plan) under another CAST or under an expression that is the first operand of an outer CAST can fail on older servers because CastTypeAliasRewriter never visited those inner CASTs before dispatch.

I'm not sure I follow this - CastTypeAliasRewriter doesn't even exist in 1.4 (https://github.com/apache/pinot/commits/release-1.4.0/).

@yashmayya

@rohityadav1993
Copy link
Copy Markdown
Contributor

+1, this is patch for the patch of the original bug. Anybody upgrading from 1.3 -> 1.4 will need to cherrypick #16682 and current PR.

@rsrkpatwari1234
Copy link
Copy Markdown
Contributor Author

@yashmayya Requesting your review on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something is not working as expected query Related to query processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix CastTypeAliasRewriter to recurse into nested CAST operands for SSE

7 participants