fix flags - #26780
Conversation
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? |
XuPeng-SH
left a comment
There was a problem hiding this comment.
[P1] Preserve source metadata when result columns are reordered
resultColumnProjectionAtNode treats ref.ColPos as an index into the current ProjectList. For a valid query such as SELECT unique_value, id FROM t, the final project list contains source refs in output order [ColPos=1, ColPos=0]; indexing it by those source positions returns the opposite expressions and swaps PRI/UNI (and related) MySQL ColumnDefinition flags. The existing tests only use source order, so this remains untested.
Resolve the projection by the source ref identity (RelPos + ColPos, with safe name fallback) or maintain an explicit output-to-source mapping; do not use the source ColPos as the current ProjectList index. Add a reordered-output regression test.
Location: pkg/sql/plan/build.go (resultColumnProjectionAtNode).
|
… 0807-fix-flags
aunjgr
left a comment
There was a problem hiding this comment.
Reviewed exact head 4781ed5 with all 26 checks terminal and passing. The previous JOIN blocker is closed: direct-column provenance follows JOIN projection and binding identity, chooses the correct child, and clears only nullability when an outer join null-extends that child. Reordered projections and planner-to-protocol flags are covered. No remaining merge-blocking correctness issue found.
Merge Queue Status
This pull request spent 27 minutes 50 seconds in the queue, with no time running CI. Waiting for
All conditions
ReasonThe merge conditions cannot be satisfied due to failing checks Failing checks:
HintYou may have to fix your CI before adding the pull request to the queue again. Requeued — the merge queue status continues in this comment ↓. |
Merge Queue Status
This pull request spent 40 seconds in the queue, including 3 seconds running CI. Required conditions to merge
|
What type of PR is this?
Which issue(s) this PR fixes:
issue #25122
What this PR does / why we need it: