Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue#18153
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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
CastTypeAliasRewriterto 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. |
|
@rohityadav1993 @Jackie-Jiang @yashmayya Requesting your review on this |
|
@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
I'm not sure I follow this - |
|
|
+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. |
|
@yashmayya Requesting your review on this |
Fixes #18152
After normalizing the cast target (BIGINT→LONG), recurse into operand 0 so nested CASTs and CASTs inside divide/etc. are fully rewritten.
bugbackward-incompatdependenciesmulti-stage