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

Remove the old horizon planner #14128

Closed
wants to merge 110 commits into from
Closed

Remove the old horizon planner #14128

wants to merge 110 commits into from

Conversation

systay
Copy link
Collaborator

@systay systay commented Sep 29, 2023

Description

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on the CI
  • Documentation was added or is not required

Deployment Notes

systay and others added 30 commits August 28, 2023 16:46
@systay
renamed planbuilder, operator and engine primitive
a344c3a 
Signed-off-by: Andres Taylor <[email protected]>
@systay
wip - handle EXISTS and push it down through an ApplyJoin
98be673 
Signed-off-by: Andres Taylor <[email protected]>
@systay
wip - change subquery pushing
ecb83d8 
Signed-off-by: Andres Taylor <[email protected]>
@systay
wip - semiJoin from EXISTS now planner correctly
f359ed0 
Signed-off-by: Andres Taylor <[email protected]>
@systay
more work on semijoin and exists
756e935 
Signed-off-by: Andres Taylor <[email protected]>
@systay
handle IN queries usign SemiJoins
e0f00ce 
Signed-off-by: Andres Taylor <[email protected]>
@systay
only add the planner phases that are needed for the current query
f7d01e7 
Signed-off-by: Andres Taylor <[email protected]>
@systay
remove SubQueryInner
04ddc38 
Signed-off-by: Andres Taylor <[email protected]>
@systay
clean up UncorrelatedSubquery
8aebcb9 
Signed-off-by: Andres Taylor <[email protected]>
@systay
handle uncorrelated subqueries better
9902d54 
Signed-off-by: Andres Taylor <[email protected]>
@systay
better support for PulloutValue subqueries
20e1340 
Signed-off-by: Andres Taylor <[email protected]>
@systay
move the query signature to the semantic analysis
0761a7a 
Signed-off-by: Andres Taylor <[email protected]>
@systay
use constructor
7df2fdd 
Signed-off-by: Andres Taylor <[email protected]>
@systay
handle subquery filter
347a36a 
Signed-off-by: Andres Taylor <[email protected]>
@systay
handle exists well in merging
337ec81 
Signed-off-by: Andres Taylor <[email protected]>
@systay
make merging great again
c988fad 
Signed-off-by: Andres Taylor <[email protected]>
@systay
go back to single expression AddColumn
71e19ad 
Signed-off-by: Andres Taylor <[email protected]>
@harshit-gangal @systay
feat: refactor and fix subqery merge logic
eb06afd 
Signed-off-by: Harshit Gangal <[email protected]>
@harshit-gangal @systay
feat: ignore columns for subquery merge when comparison is not equal …
3da93bc 
…operator, push ordering under subquery container, check if projection is needed on top of subqueryfilter

Signed-off-by: Harshit Gangal <[email protected]>
@harshit-gangal @systay
fix: set pullout variables on operator to set them on engine primitiv…
60dddd5 
…e on transform

Signed-off-by: Harshit Gangal <[email protected]>
@harshit-gangal @systay
feat: push filter on subquery filter
7800ca9 
Signed-off-by: Harshit Gangal <[email protected]>
@systay
handle pushing subqueries when the subquery depends on both sides of …
f26eb69 
…a join

Signed-off-by: Andres Taylor <[email protected]>
@systay
update some plantests
24ab078 
Signed-off-by: Andres Taylor <[email protected]>
@systay
push projections under subqueries
876f557 
Signed-off-by: Andres Taylor <[email protected]>
@systay
move the subquery-filter to be closer to the ApplyJoin
1bc2b4c 
Signed-off-by: Andres Taylor <[email protected]>
@systay
make it possible to push aggregation through subquery
ca8e7c9 
Signed-off-by: Andres Taylor <[email protected]>
@systay
lots of work on recursive subqueries
8f2e397 
Signed-off-by: Andres Taylor <[email protected]>
@harshit-gangal
Merge remote-tracking branch 'upstream/main' into subq-op
126a37f 
Signed-off-by: Harshit Gangal <[email protected]>
@harshit-gangal
fail for cases when correlated subquery is not an exists query
8d7811f 
Signed-off-by: Harshit Gangal <[email protected]>
@systay
calculate the join columns and outer columns needed on demand
8055ea7 
Signed-off-by: Andres Taylor <[email protected]>
harshit-gangal and others added 23 commits September 22, 2023 11:50
@harshit-gangal
create subquery planner for having clause
155dc02 
Signed-off-by: Harshit Gangal <[email protected]>
@systay
Merge remote-tracking branch 'upstream/main' into subq-op
6cc5a6d 
Signed-off-by: Andres Taylor <[email protected]>
@systay
last couple of tests turned green
792ce30 
Signed-off-by: Andres Taylor <[email protected]>
@systay
check that subqueries used in comparisons have the correct number of …
a89e3d5 
…columns

Signed-off-by: Andres Taylor <[email protected]>
@systay
remove column number check from engine primitive and remove outdated …
f9705b4 
…tests

Signed-off-by: Andres Taylor <[email protected]>
@systay
restore the space between exists and the parens
b756846 
Signed-off-by: Andres Taylor <[email protected]>
@systay
remove invalid queries
df4b8fa 
Signed-off-by: Andres Taylor <[email protected]>
@systay
make sure the engine primitive handles the new plans correctly
ba8ea10 
Signed-off-by: Andres Taylor <[email protected]>
@systay
comments and renaming for clarification
9ae46d1 
Signed-off-by: Andres Taylor <[email protected]>
@systay
refactoring & remove silly test
bcb657d 
Signed-off-by: Andres Taylor <[email protected]>
@systay
Rename pushDown to push in the operator package
e66336a 
My thinking is that `Down` is implied. We never "push up", so
it is redundant to say pushDown everywhere.

Signed-off-by: Andres Taylor <[email protected]>
@systay
remove another query with subq in outer join condition
7ab4d8c 
Signed-off-by: Andres Taylor <[email protected]>
@systay
bug: check the output columns on commented queries
894823e 
Signed-off-by: Andres Taylor <[email protected]>
@GuptaManan100
test: fix test expectation and add a comment explaining it
a104a2d 
Signed-off-by: Manan Gupta <[email protected]>
@GuptaManan100
feat: fix pushing order by underneath an aggregation
173ea54 
Signed-off-by: Manan Gupta <[email protected]>
@systay
bug: fix the subquery merging logic
eb23151 
Signed-off-by: Andres Taylor <[email protected]>
@systay
address review comments
baeeabd 
Signed-off-by: Andres Taylor <[email protected]>
@systay
extract subquery building from subquery container
b804997 
Signed-off-by: Andres Taylor <[email protected]>
@systay
allow merging but not routing if predicates are deep in expression tree
5f4b40d 
Signed-off-by: Andres Taylor <[email protected]>
@systay
clean up projection subquery planning
b513c99 
Signed-off-by: Andres Taylor <[email protected]>
@systay
Merge remote-tracking branch 'upstream/main' into subq-op
02afec8 
Signed-off-by: Andres Taylor <[email protected]>
@harshit-gangal @systay
handle subquery with vindex value on update better with blocking merge
826a3bb 
Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
@systay
make sure to handle Exists in projections correctly
77e4cb7 
Signed-off-by: Andres Taylor <[email protected]>
@vitess-bot
Copy link
Contributor

vitess-bot bot commented Sep 29, 2023

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 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 Sep 29, 2023
@github-actions github-actions bot added this to the v18.0.0 milestone Sep 29, 2023
@systay systay closed this Sep 29, 2023
@harshit-gangal harshit-gangal deleted the subq-op branch December 21, 2023 09:04
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

@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

@rohit-nayak-ps rohit-nayak-ps Awaiting requested review from rohit-nayak-ps rohit-nayak-ps will be requested when the pull request is marked ready for review rohit-nayak-ps 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

@shlomi-noach shlomi-noach Awaiting requested review from shlomi-noach shlomi-noach will be requested when the pull request is marked ready for review shlomi-noach 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

@frouioui frouioui Awaiting requested review from frouioui frouioui will be requested when the pull request is marked ready for review frouioui 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

Assignees
No one assigned
Labels
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
v18.0.0
Development

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