Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for value joins #17491

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Add support for value joins #17491

wants to merge 12 commits into from

Conversation

frouioui
Copy link
Member

@frouioui frouioui commented Jan 8, 2025

Description

This PR builds on top of #16703, most of the commits were cherry-picked, including:

  • AST support for VALUES statements
  • SQL parsing support
  • Operator
  • Engine primitive

Related Issue(s)

harshit-gangal and others added 12 commits January 8, 2025 10:37
@harshit-gangal @frouioui
feat: added values statement to ast
bc60310 
Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@harshit-gangal @frouioui
feat: add support for values statement with list argument bind variab…
1be21ef 
…le. Added new Row_Tuple bind variable type for formatting in vttablet

Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@systay @frouioui
minor tweaks
9ecaf2b 
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@GuptaManan100 @frouioui
feat: fix parsing for values
1262cbb 
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@frouioui
Add ValuesJoin Operator
ed0136f 
Signed-off-by: Florent Poinsard <[email protected]>
@frouioui
Simplify code
5e2b4a1 
Signed-off-by: Florent Poinsard <[email protected]>
@systay @frouioui
more planning support
6869f72 
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@systay @frouioui
only use ValuesJoin when MySQL supports it
c7f4001 
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@systay @frouioui
instead of rewriting applyJoin to valuesJoin, we'll start with either…
de7f537 
… form and stay there

Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@systay @frouioui
only report the expressions coming from the RHS
b73aded 
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@harshit-gangal @frouioui
push values table under route and add values table to values join
f584098 
Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@systay @frouioui
rename engine primitive
d25b47e 
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@vitess-bot vitess-bot
Copy link
Contributor

vitess-bot bot commented Jan 8, 2025

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 8, 2025
@github-actions github-actions bot added this to the v22.0.0 milestone Jan 8, 2025
@harshit-gangal
Copy link
Member

I just created a small PR out of it, so that planner changes can be a separate PR
#17500

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepthi deepthi Awaiting requested review from deepthi deepthi will be requested when the pull request is marked ready for review deepthi is a code owner

@harshit-gangal harshit-gangal Awaiting requested review from harshit-gangal harshit-gangal will be requested when the pull request is marked ready for review harshit-gangal is a code owner

@mattlord mattlord Awaiting requested review from mattlord mattlord will be requested when the pull request is marked ready for review mattlord is a code owner

@systay systay Awaiting requested review from systay systay will be requested when the pull request is marked ready for review systay is a code owner

@GuptaManan100 GuptaManan100 Awaiting requested review from GuptaManan100 GuptaManan100 will be requested when the pull request is marked ready for review GuptaManan100 is a code owner

@arthurschreiber arthurschreiber Awaiting requested review from arthurschreiber arthurschreiber will be requested when the pull request is marked ready for review arthurschreiber is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
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
Projects
None yet
Milestone
v22.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@frouioui @harshit-gangal @systay @GuptaManan100