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

Fix for empty results when no shards can be found to route to #10152

Merged
merged 8 commits into from
Apr 29, 2022
Merged

Fix for empty results when no shards can be found to route to #10152

merged 8 commits into from
Apr 29, 2022

Conversation

GuptaManan100
Copy link
Member

@GuptaManan100 GuptaManan100 commented Apr 27, 2022 •
edited by systay
Loading

Description

Queries that follow the following pattern -

select exists(select * from foo where some_field=some_value)

The table foo would has a lookup vindex on some_field and there are no matching rows for some_value. What Vitess would do when no shards could be found to route to is to return an empty result. This is correct for most cases, but not always. The query above would return an empty result instead of a 0.

Related Issue(s)

Checklist

  • "Backport me!" label has been added if this change should be backported YES
  • Tests were added or are not required
  • Documentation was added or is not required

@GuptaManan100
feat: prevent merging of subqueries if the outer query is a reference
8b0e22f 
Signed-off-by: Manan Gupta <manan@planetscale.com>
@GuptaManan100
test: added tests for subquery with reference
d538134 
Signed-off-by: Manan Gupta <manan@planetscale.com>
@GuptaManan100
feat: send query to any shard incase there are no shards which contai…
104de0b 
…n results for that query

Signed-off-by: Manan Gupta <manan@planetscale.com>
@GuptaManan100
test: added tests for other subquery cases
6f070af 
Signed-off-by: Manan Gupta <manan@planetscale.com>
@GuptaManan100 GuptaManan100 mentioned this pull request Apr 27, 2022
Closed
@systay
test: update unit tests to reflect new behaviour
f1ea142 
Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay
feat: handle streaming in routing when no shards are found
3f5a894 
Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay changed the title Fix for subquery in a Reference Query Fix for empty results when no shards can be found to route to Apr 28, 2022
@systay
feat: only use the special no routes handling when the route is the o…
9d444b1 
…nly primitive

Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay marked this pull request as ready for review April 28, 2022 08:23
GuptaManan100
GuptaManan100 commented Apr 28, 2022
View reviewed changes
Copy link
Member Author

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

Everything else LGTM!

go/vt/vtgate/planbuilder/gen4_planner.go Show resolved Hide resolved
@systay
doc: added helpful comment
ce05a28 
Signed-off-by: Andres Taylor <andres@planetscale.com>
systay
systay approved these changes Apr 29, 2022
View reviewed changes
Copy link
Collaborator

@systay systay left a comment

Choose a reason for hiding this comment

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

LGTM (Looks Good To Manan)

@systay systay merged commit 8a5950d into vitessio:main Apr 29, 2022
@systay systay deleted the exists-bug branch April 29, 2022 06:39
systay pushed a commit to planetscale/vitess that referenced this pull request May 2, 2022
@GuptaManan100 @systay
Fix for empty results when no shards can be found to route to (vitess…
52e9c3e 
…io#10152)

* feat: prevent merging of subqueries if the outer query is a reference

Signed-off-by: Manan Gupta <manan@planetscale.com>

* test: added tests for subquery with reference

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: send query to any shard incase there are no shards which contain results for that query

Signed-off-by: Manan Gupta <manan@planetscale.com>

* test: added tests for other subquery cases

Signed-off-by: Manan Gupta <manan@planetscale.com>

* test: update unit tests to reflect new behaviour

Signed-off-by: Andres Taylor <andres@planetscale.com>

* feat: handle streaming in routing when no shards are found

Signed-off-by: Andres Taylor <andres@planetscale.com>

* feat: only use the special no routes handling when the route is the only primitive

Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay mentioned this pull request May 2, 2022
Merged
systay added a commit that referenced this pull request May 2, 2022
@systay @GuptaManan100 @frouioui
Fix for empty results when no shards can be found to route to (#10187)
6f40aaa 
* Fix for empty results when no shards can be found to route to (#10152)

* feat: prevent merging of subqueries if the outer query is a reference

Signed-off-by: Manan Gupta <manan@planetscale.com>

* test: added tests for subquery with reference

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: send query to any shard incase there are no shards which contain results for that query

Signed-off-by: Manan Gupta <manan@planetscale.com>

* test: added tests for other subquery cases

Signed-off-by: Manan Gupta <manan@planetscale.com>

* test: update unit tests to reflect new behaviour

Signed-off-by: Andres Taylor <andres@planetscale.com>

* feat: handle streaming in routing when no shards are found

Signed-off-by: Andres Taylor <andres@planetscale.com>

* feat: only use the special no routes handling when the route is the only primitive

Signed-off-by: Andres Taylor <andres@planetscale.com>

* Skip TestSubqueryInReference if the vtgate version is below 14

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

* Skip TestSubqueryInReference if the vtgate version is below 13

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

Co-authored-by: Manan Gupta <35839558+GuptaManan100@users.noreply.github.com>
Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr>
@systay systay mentioned this pull request Jun 3, 2022
Merged
3 tasks
@harshit-gangal harshit-gangal mentioned this pull request Aug 16, 2022
Merged
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 Awaiting requested review from harshit-gangal harshit-gangal is a code owner

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

Assignees
No one assigned
Labels
Component: Query Serving Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug Report: exists may returns random result on a sharded table
2 participants
@GuptaManan100 @systay