You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "rsrkpatwari1234 (via GitHub)" <gi...@apache.org> on 2026/04/09 16:24:42 UTC
[PR] Allow CastTypeAliasRewriter to recurse into nested CAST operands to f… [pinot]
rsrkpatwari1234 opened a new pull request, #18153:
URL: https://github.com/apache/pinot/pull/18153
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`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue [pinot]
Posted by "rsrkpatwari1234 (via GitHub)" <gi...@apache.org>.
rsrkpatwari1234 commented on PR #18153:
URL: https://github.com/apache/pinot/pull/18153#issuecomment-4229472982
> @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
- We are upgrading pinot from 1.3 -> 1.4 and currently at stage where controllers + brokers are at 1.4 and servers at 1.3
- We cherry-picked #16682 on top of pinot release-1.4.0 to fix issue - `(IllegalArgumentException: Unable to cast expression to type - BIGINT in CastTransformFunction.init)`
- Even with #16682, we saw multiple failures for our critical queries since nested CAST is not handled as part of above.
- Hence, we need a fix for this to continue upgrade rollouts and ensure backward compatibility
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue [pinot]
Posted by "rsrkpatwari1234 (via GitHub)" <gi...@apache.org>.
rsrkpatwari1234 commented on PR #18153:
URL: https://github.com/apache/pinot/pull/18153#issuecomment-4243128574
@yashmayya Requesting your review on this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue [pinot]
Posted by "rsrkpatwari1234 (via GitHub)" <gi...@apache.org>.
rsrkpatwari1234 commented on PR #18153:
URL: https://github.com/apache/pinot/pull/18153#issuecomment-4219512992
@rohityadav1993 @Jackie-Jiang @yashmayya Requesting your review on this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue [pinot]
Posted by "rohityadav1993 (via GitHub)" <gi...@apache.org>.
rohityadav1993 commented on PR #18153:
URL: https://github.com/apache/pinot/pull/18153#issuecomment-4231348552
+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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya merged PR #18153:
URL: https://github.com/apache/pinot/pull/18153
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18153:
URL: https://github.com/apache/pinot/pull/18153#issuecomment-4216526687
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18153?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 55.55%. Comparing base ([`dcbe2ae`](https://app.codecov.io/gh/apache/pinot/commit/dcbe2aead8ce76b989cede9c7d31309c8e3784d9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8b7a3d7`](https://app.codecov.io/gh/apache/pinot/commit/8b7a3d7bf5722c53561b615ea1dfa5316764851e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (dcbe2ae) and HEAD (8b7a3d7). Click for more details.
>
> <details><summary>HEAD has 36 uploads less than BASE</summary>
>
>| Flag | BASE (dcbe2ae) | HEAD (8b7a3d7) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|4|1|
>|temurin|10|1|
>|java-11|5|0|
>|unittests2|2|0|
>|integration|6|0|
>|integration2|2|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18153 +/- ##
============================================
- Coverage 63.03% 55.55% -7.48%
+ Complexity 1617 822 -795
============================================
Files 3202 2507 -695
Lines 194717 143697 -51020
Branches 30046 23053 -6993
============================================
- Hits 122739 79833 -42906
+ Misses 62250 57025 -5225
+ Partials 9728 6839 -2889
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.55% <100.00%> (-7.45%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.55% <100.00%> (-7.48%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.55% <100.00%> (-7.48%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.55% <100.00%> (-0.01%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/18153?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Allow CastTypeAliasRewriter to recurse into nested CAST operands to fix upgrade incompatibility issue [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on PR #18153:
URL: https://github.com/apache/pinot/pull/18153#issuecomment-4225609862
@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/).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org