Skip to content

Fix subquery cloning and dependencies#15039

Merged
harshit-gangal merged 2 commits intovitessio:mainfrom
planetscale:fix-deps-of-subquery
Jan 29, 2024
Merged

Fix subquery cloning and dependencies#15039
harshit-gangal merged 2 commits intovitessio:mainfrom
planetscale:fix-deps-of-subquery

Conversation

@frouioui
Copy link
Copy Markdown
Member

@frouioui frouioui commented Jan 25, 2024 •
edited by systay
Loading

Description

When planning sharded queries, we were not handling expressions in the ORDER BY correctly.
If subqueries were used in the ORDER BY clause, we would not expand them correctly, and that lead to weird problems.

Another issue we encountered was cloning of ORDER BY expressions. When we do that without also copying semantic information, we incorrectly thought that non-correlated queries were correlated.

Related Issue(s)

Fixes #15050

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

@vitess-bot
Copy link
Copy Markdown
Contributor

vitess-bot bot commented Jan 25, 2024

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.

@vitess-bot vitess-bot bot added NeedsBackportReason If backport labels have been applied to a PR, a justification is required NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsIssue A linked issue is missing for this Pull Request NeedsWebsiteDocsUpdate What it says labels Jan 25, 2024
@github-actions github-actions bot added this to the v19.0.0 milestone Jan 25, 2024
@frouioui frouioui added Type: Bug Component: Query Serving and removed NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsWebsiteDocsUpdate What it says 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 Jan 25, 2024
frouioui and others added 2 commits January 27, 2024 11:00
@frouioui @systay
Copy the dependencies of the subquery properly during clonning
5652d3f 
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@systay
feat: make sure to handle subqueries correctly planning ORDER BY
7dbabd3 
Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay force-pushed the fix-deps-of-subquery branch from cdbfcf3 to 7dbabd3 Compare January 27, 2024 10:00
@systay systay marked this pull request as ready for review January 27, 2024 10:03
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 27, 2024

Codecov Report

Attention: 39 lines in your changes are missing coverage. Please review.

Comparison is base (44d6a6b) 47.49% compared to head (7dbabd3) 47.49%.

Files Patch % Lines
.../vtgate/planbuilder/operators/horizon_expanding.go 0.00% 29 Missing ⚠️
go/vt/vtgate/semantics/semantic_state.go 0.00% 7 Missing ⚠️
...vt/vtgate/planbuilder/operators/queryprojection.go 0.00% 1 Missing ⚠️
go/vt/vtgate/planbuilder/operators/subquery.go 0.00% 1 Missing ⚠️
...t/vtgate/planbuilder/operators/subquery_builder.go 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #15039      +/-   ##
==========================================
- Coverage   47.49%   47.49%   -0.01%     
==========================================
  Files        1149     1149              
  Lines      239387   239405      +18     
==========================================
+ Hits       113692   113697       +5     
- Misses     117102   117121      +19     
+ Partials     8593     8587       -6

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

@harshit-gangal harshit-gangal merged commit fd99639 into vitessio:main Jan 29, 2024
@harshit-gangal harshit-gangal deleted the fix-deps-of-subquery branch January 29, 2024 14:57
@vitess-bot vitess-bot bot mentioned this pull request Jan 29, 2024
Closed
@systay systay restored the fix-deps-of-subquery branch June 3, 2024 11:27
@systay systay mentioned this pull request Jun 11, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@systay systay systay approved these changes

@harshit-gangal harshit-gangal harshit-gangal approved these changes

@GuptaManan100 GuptaManan100 Awaiting requested review from GuptaManan100

@arthurschreiber arthurschreiber Awaiting requested review from arthurschreiber arthurschreiber is a code owner

Assignees

No one assigned

Labels

Component: Query Serving Type: Bug

Projects

None yet

Milestone

v19.0.0

Development

Successfully merging this pull request may close these issues.

Planner bug: Subquery incorrectly labeled as correlated

3 participants

@frouioui @systay @harshit-gangal