Skip to content

Conversation

arthurschreiber
Copy link

Description

This is a backport of vitessio#18260

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

@Copilot Copilot AI review requested due to automatic review settings May 12, 2025 08:35
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Backports the fix for subquery merging regression introduced in issue vitessio#11379, as implemented in vitessio#18260.

  • Updates expected planbuilder test cases to remove deprecated UncorrelatedSubquery instructions and adjust FieldQuery/Query fields
  • Refines canMergeOnFilter to extract only column names using getColName before vindex lookup

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
go/vt/vtgate/planbuilder/testdata/select_cases.json Adjusted JSON test expectations for subquery routes
go/vt/vtgate/planbuilder/operators/route_planning.go Switched comparison.Left/Right to getColName calls in canMergeOnFilter
Comments suppressed due to low confidence (2)

go/vt/vtgate/planbuilder/operators/route_planning.go:395

  • [nitpick] The variables left and right now hold *sqlparser.ColName values or nil. Consider renaming them to leftCol and rightCol for clarity.
left := getColName(comparison.Left)

go/vt/vtgate/planbuilder/operators/route_planning.go:395

  • Add unit tests for canMergeOnFilter covering cases where comparison.Left or comparison.Right is not a simple column (e.g., aggregate expressions), to ensure getColName returning nil is handled correctly.
left := getColName(comparison.Left)

@arthurschreiber arthurschreiber merged commit 498a466 into release-19.0-github May 12, 2025
195 of 201 checks passed
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.

2 participants