fix flatten regression#4197
Merged
Merged
Conversation
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4197 +/- ##
==========================================
- Coverage 95.41% 95.16% -0.26%
==========================================
Files 171 171
Lines 43786 43788 +2
Branches 7502 7503 +1
==========================================
- Hits 41778 41670 -108
- Misses 1227 1295 +68
- Partials 781 823 +42 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sfc-gh-mayliu
approved these changes
Apr 23, 2026
sfc-gh-yixie
approved these changes
Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-NNNNNNN
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Commit 07689e0 ("Loosen flattening rules for sort and filter") loosened can_clause_dependent_columns_flatten so that, when both _is_snowpark_connect_compatible_mode and _snowpark_connect_flatten_select_after_sort are True, a filter depending on a NEW (e.g. aliased) column in a subquery is allowed to be flattened.
That loosening is correct for sort / select / distinct, but it is unsound for filter across LIMIT/OFFSET. Flattening pushes the WHERE in front of the LIMIT, which changes the result set:
Repro SQL on Snowpark Connect: SELECT * FROM (SELECT * FROM range(10) LIMIT 5) WHERE id > 3 used to return [4], now returns [4, 5, 6, 7, 8].
Fix
Narrow the loosening that 07689e0 introduced instead of reverting it. Inside the NEW-column branch of can_clause_dependent_columns_flatten — which is only reachable when both compat-mode flags are True — add a single guard that rejects flattening when clause == "filter" and the subquery has LIMIT or OFFSET:
The helper gains one optional kwarg, subquery_has_limit_or_offset: bool = False, passed in from SelectStatement.filter. SelectStatement.sort already short-circuits on not self.limit_ / not self.offset at the call site, so no change is needed there.
Is this a BCR?
No. Existing Snowpark customers (legacy mode) observe no change in generated SQL. The only behavioral delta is for Snowpark Connect / compatible-mode callers on the exact pattern the bad commit newly enabled — where we are restoring pre-commit semantics.