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.

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

[kvexec] fix more lookup bugs related to schema/projection inconsistencies #8311

Merged
merged 6 commits into from
Aug 30, 2024

Conversation

max-hoffman
Copy link
Contributor

@max-hoffman max-hoffman commented Aug 29, 2024

We were using the full table schema to index a covering index KV pair. The fix has to be sensitive to covering, non-covering, and keyless tables, which required extra tests on the GMS side to check all cases.

fixes: #8308
re: dolthub/go-mysql-server#2646

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

Code looks fine, but where are the tests?

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

comparing_percentages
100.000000 to 100.000000
version result total
0f40db9 ok 5937457
version total_tests
0f40db9 5937457
correctness_percentage
100.0

@max-hoffman
Copy link
Contributor Author

Code looks fine, but where are the tests?

GMS side, linked in description dolthub/go-mysql-server#2646

@coffeegoddd
Copy link
Contributor

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
485313c ok 5937457
version total_tests
485313c 5937457
correctness_percentage
100.0

@max-hoffman max-hoffman merged commit d2ac2de into main Aug 30, 2024
21 checks passed
@max-hoffman max-hoffman deleted the max/kv-lookup-bugs branch August 30, 2024 00:11
@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

comparing_percentages
100.000000 to 100.000000
version result total
5481ace ok 5937457
version total_tests
5481ace 5937457
correctness_percentage
100.0

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.

Empty result set from query with INNER JOIN + WHERE
3 participants