Skip to content

VTGate: Route single-valued information_schema IN predicates like equality - #20973

Open
mattlord wants to merge 17 commits into
mainfrom
i_s-single_element_in
Open

VTGate: Route single-valued information_schema IN predicates like equality#20973
mattlord wants to merge 17 commits into
mainfrom
i_s-single_element_in

Conversation

@mattlord

@mattlord mattlord commented Sep 2, 2026

Copy link
Copy Markdown
Member

Description

information_schema queries using an IN predicate on table_schema/table_name bypassed schema-name routing entirely and silently returned empty or incomplete results — the query shape Rails now generates for all of its schema readers.

The planner now treats a single-valued IN as the equality it is: a one-element literal list is unwrapped at plan time, and a bound list (what vtgate's normalizer produces — its length is unknown until execution) is extracted and resolved at execution time, where one value routes exactly like = and anything else fails with an explicit VT12001 error instead of silently querying the wrong keyspace. Multi-value table_name IN lists are deliberately left as pushed-down filters, since they already work correctly once the schema routes. No normalizer changes were needed, so one cached plan serves any list length.

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported to release branches
  • If this change is to be back-ported to previous releases, a justification is included in the PR description
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on CI?
  • Documentation was added or is not required

Deployment Notes

AI Disclosure

I used Claude with Fable5 for the work and Codex with GPT-5.6 for reviews.

…quality

Signed-off-by: Matt Lord <mattalord@gmail.com>
…e columns

Signed-off-by: Matt Lord <mattalord@gmail.com>
… predicates

Signed-off-by: Matt Lord <mattalord@gmail.com>
…time

Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
…n error and changelog wording, add characterization pins

- planner: run the evalengine.Translate pre-check on a single-element IN's
  tuple element before rewriting it to `=`, so an untranslatable element
  (e.g. table_schema in (other_col)) leaves the IN untouched instead of
  being cosmetically rewritten while extraction still fails
- engine: VT12001 message now covers both zero-length and multi-value
  tuples ("other than one schema name")
- changelog: qualify the multi-value error sentence to the bound/normalized
  IN list form
- testdata: pin untranslatable single-element IN (red before the planner
  fix, green after), NOT IN (already correct), and non-routing-column
  ListArg (already correct) in both info_schema80/57_cases.json

Signed-off-by: Matt Lord <mattalord@gmail.com>
Copilot AI balanced review requested due to automatic review settings September 2, 2026 01:22
@mattlord mattlord added Type: Bug Component: Query Serving Backport to: release-23.0 Needs to be backport to release-23.0 Backport to: release-24.0 Needs to be backport to release-24.0 labels Sep 2, 2026
@github-actions github-actions Bot added this to the v25.0.0 milestone Sep 2, 2026
@vitess-bot vitess-bot Bot added NeedsWebsiteDocsUpdate What it says NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsIssue A linked issue is missing for this Pull Request NeedsBackportReason If backport labels have been applied to a PR, a justification is required labels Sep 2, 2026
@vitess-bot

vitess-bot Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

  • Ensure that the Pull Request has a descriptive title.
  • Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

  • Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

  • Apply the release notes (needs details) label if users need to know about this change.
  • New features should be documented.
  • There should be some code comments as to why things are implemented the way they are.
  • There should be a comment at the top of each new or modified test to explain what the test does.

New flags

  • Is this flag really necessary?
  • Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

  • Each item in Jobs should be named in order to mark it as required.
  • If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

  • Protobuf changes should be wire-compatible.
  • Changes to _vt tables and RPCs need to be backward compatible.
  • RPC changes should be compatible with vitess-operator
  • If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
  • vtctl command output order should be stable and awk-able.

@mattlord mattlord removed NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsIssue A linked issue is missing for this Pull Request NeedsBackportReason If backport labels have been applied to a PR, a justification is required NeedsWebsiteDocsUpdate What it says labels Sep 2, 2026

This comment was marked as resolved.

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.06%. Comparing base (70c7a72) to head (bfe7b76).
⚠️ Report is 530 commits behind head on main.

Files with missing lines Patch % Lines
...gate/planbuilder/operators/info_schema_planning.go 95.55% 2 Missing ⚠️
go/vt/vtgate/engine/routing.go 98.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main   #20973       +/-   ##
===========================================
+ Coverage   69.67%   77.06%    +7.39%     
===========================================
  Files        1614      553     -1061     
  Lines      216793    84400   -132393     
===========================================
- Hits       151044    65046    -85998     
+ Misses      65749    19354    -46395     
Flag Coverage Δ
partial 77.06% <97.14%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…e-name table_name lists

Signed-off-by: Matt Lord <mattalord@gmail.com>
Copilot AI review requested due to automatic review settings September 2, 2026 01:54
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Signed-off-by: Matt Lord <mattalord@gmail.com>
Copilot AI review requested due to automatic review settings September 4, 2026 01:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The execution-time routing changes warrant final human review despite comprehensive test coverage.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

devin-ai-integration[bot]

This comment was marked as resolved.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Signed-off-by: Matt Lord <mattalord@gmail.com>
Copilot AI review requested due to automatic review settings September 4, 2026 01:51

This comment was marked as resolved.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Copilot AI review requested due to automatic review settings September 4, 2026 02:12

This comment was marked as resolved.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord
mattlord force-pushed the i_s-single_element_in branch from 8b9a2cc to 7e88e30 Compare September 4, 2026 02:21
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Signed-off-by: Matt Lord <mattalord@gmail.com>
Copilot AI review requested due to automatic review settings September 4, 2026 02:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Runtime routing and planner changes warrant final human review.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Signed-off-by: Matt Lord <mattalord@gmail.com>
Copilot AI review requested due to automatic review settings September 4, 2026 02:48

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

Devin Review

Comment on lines +105 to +106
// The column's bind variable already carries a different value; a
// second one cannot share it, so this predicate stays a plain filter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Unrequested explanatory comment

The new explanation violates the repository rule against adding explanatory comments unless requested.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The critical multi-schema database() routing case must be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Balanced

}
// Multi-element schema lists are resolved at execution. table_name
// lists already work as pushed-down filters.
if !isSchema || !translates(rhs) || slices.ContainsFunc(rhs, func(e sqlparser.Expr) bool { return !shouldRewrite(e) }) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug Report: I_S IN queries get routed to the default keyspace, can return empty rows or incomplete answers

3 participants