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

Update logic to determine whether a LookUp call with a reduction formula argument can be delegated #2065

Merged
merged 13 commits into from
Mar 14, 2024

Conversation

ian-legler
Copy link
Contributor

Currently, the logic to determine whether a LookUp function call with a reduction formula argument is incorrect, purely checking whether the node is marked as NOT delegatable.

This means that most scenarios are marked delegable, but uses of the LookUp function as part of the filter in a delegatable function (e.g. LookUp, Filter, etc.) can lead to incorrect code gen.

Updating the logic to try to determine whether the full function can be delegated, in context.

@LucGenetier
Copy link
Contributor

✅ No public API change.

@LucGenetier
Copy link
Contributor

✅ No public API change.

@LucGenetier
Copy link
Contributor

✅ No public API change.

@LucGenetier
Copy link
Contributor

✅ No public API change.

@ian-legler ian-legler marked this pull request as ready for review February 29, 2024 21:35
@ian-legler ian-legler requested a review from a team as a code owner February 29, 2024 21:35
@LucGenetier
Copy link
Contributor

✅ No public API change.

@LucGenetier
Copy link
Contributor

✅ No public API change.

@LucGenetier
Copy link
Contributor

✅ No public API change.

@LucGenetier
Copy link
Contributor

✅ No public API change.

@ian-legler ian-legler merged commit 68f1414 into main Mar 14, 2024
3 of 4 checks passed
ian-legler added a commit that referenced this pull request Mar 15, 2024
Incorrect logic for determining whether a LookUp Call node could be
delegated was added in a previous PR
#2065

Updating the logic to ensure proper handling of edge cases
Sign up for free to 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.

4 participants