fix(plan): dedup PostgreSQL-style UPDATE ... FROM by target row_id - #25023
Merged
Conversation
Port of PR matrixorigin#24932 (issue matrixorigin#23137) to main. When the source of an UPDATE ... FROM matches a target row more than once, the target row was duplicated/updated multiple times. Dedup the duplicate source matches with a row_number() window partitioned on the target row's physical identity (row_id) instead of the whole old target row: partitioning on every old column would crash on GEOMETRY32 (no comparator), miss dedup on float columns holding NaN (NaN != NaN), and wrongly merge distinct rows whose columns happen to match. The join-target NULL-row filter (isnotnull(row_id)) is preserved on the fallback path. Adds the missing T_Rowid (and other fixed-size type) cases to pkg/partition so the row_id partition key does not panic at runtime. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ck89119
requested review from
aunjgr,
heni02,
iamlinjunhong and
ouyuanning
as code owners
June 17, 2026 04:19
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
heni02
approved these changes
Jun 17, 2026
# Conflicts: # pkg/sql/plan/build_test.go
XuPeng-SH
approved these changes
Jun 17, 2026
aunjgr
approved these changes
Jun 18, 2026
Contributor
Merge Queue Status
This pull request spent 1 hour 11 minutes 32 seconds in the queue, including 1 hour 11 minutes 15 seconds running CI. Required conditions to merge
|
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
What type of PR is this?
Which issue(s) this PR fixes:
issue #23137
What this PR does / why we need it:
Port of #24932 (targeting
4.0-dev) tomain.When the source of a PostgreSQL-style
UPDATE ... FROMmatches a target row more than once, the target row was duplicated/updated multiple times. This PR dedups the duplicate source matches with arow_number()window partitioned on the target row's physical identity (row_id) instead of on the whole old target row.Partitioning on every old target column was unsafe:
GEOMETRY32(no comparator inpkg/compare),NaN(NaN != NaN),The
row_idkey is stable, has a comparator, has noNaN, and is physically unique.Additional changes:
isnotnull(row_id)) is preserved on the fallback (buildTableUpdate) path even after the dedup is switched fromany_valueaggregation to therow_number()window.pkg/partitiongains the missingT_Rowid(and other fixed-size type) cases so therow_idpartition key does not panic at runtime.