Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEA] Remove isFoldableNonLitAllowed toggle #10887

Open
binmahone opened this issue May 24, 2024 · 1 comment
Open

[FEA] Remove isFoldableNonLitAllowed toggle #10887

binmahone opened this issue May 24, 2024 · 1 comment
Assignees
Labels
feature request New feature or request

Comments

@binmahone
Copy link
Collaborator

binmahone commented May 24, 2024

from Bobby:

I'm not sure we need this at all. The foldable check was put in place a long time ago when each expression had to have explicit support for scalar values. We refectored the code a while ago and removed that. Just the fact that it works for testing means that the refactor was correct and I think we can just delete the check entirely.

Details explained at https://github.com/NVIDIA/spark-rapids/pull/10851/files/7237cb601ff9e8097deab6e95c968f6cb4c863a7#r1608257029

@binmahone binmahone added feature request New feature or request ? - Needs Triage Need team to review and classify labels May 24, 2024
@binmahone binmahone changed the title [FEA] Remove FOLDABLE_NON_LIT_ALLOWED [FEA] Remove isFoldableNonLitAllowed toggle May 24, 2024
@binmahone binmahone self-assigned this May 24, 2024
@revans2
Copy link
Collaborator

revans2 commented May 24, 2024

To be clear in order to remove isFoldableNonLitAllowed. We need to remove the check for the foldable config entirely. We fixed the issue related to it a long time ago, but we never updated the code to ignore it. It would be good to disable folding for some of the integration tests, like some binary operations, and add in a few tests that would pass in all literal parameters to validate that those expressions are doing the right thing.

@mattahrens mattahrens removed the ? - Needs Triage Need team to review and classify label May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants