Skip to content

Return nil lookupMapping if type conversion is not in range for lookup key#10248

Merged
angelamayxie merged 7 commits intomainfrom
angela/key_conversion
Dec 30, 2025
Merged

Return nil lookupMapping if type conversion is not in range for lookup key#10248
angelamayxie merged 7 commits intomainfrom
angela/key_conversion

Conversation

@angelamayxie
Copy link
Copy Markdown
Contributor

@angelamayxie angelamayxie commented Dec 29, 2025

fixes #10233

Not checking if a value is in the converted range was causing us to use incorrect keys. For example, if the literal -87840 was used as a key for a boolean/tinyint column, it would get converted to -128 and match rows with -128, even though that's not the same value.

This change checks for if a conversion is InRange, and if it's not, it returns a nil lookupMapping. The nil lookupMapping will then return false when *lookupMapping.valid() is called.

Test added in dolthub/go-mysql-server#3357

@coffeegoddd
Copy link
Copy Markdown
Contributor

@angelamayxie DOLT

comparing_percentages
100.000000 to 100.000000
version result total
9237892 ok 5937471
version total_tests
9237892 5937471
correctness_percentage
100.0

@angelamayxie angelamayxie changed the title Fall back to literal value if type conversion is not in range Fall back to literal value if type conversion is not in range for lookup key Dec 30, 2025
@coffeegoddd
Copy link
Copy Markdown
Contributor

@angelamayxie DOLT

comparing_percentages
100.000000 to 100.000000
version result total
37519e2 ok 5937471
version total_tests
37519e2 5937471
correctness_percentage
100.0

@angelamayxie angelamayxie marked this pull request as ready for review December 30, 2025 18:16
Copy link
Copy Markdown
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.

Probably should use the same logic for this that GMS does, this seems like it was just an oversight when the latest round of bug fixes for this problem were just applied ther.

@angelamayxie angelamayxie changed the title Fall back to literal value if type conversion is not in range for lookup key Return nil lookupMapping if type conversion is not in range for lookup key Dec 30, 2025
@coffeegoddd
Copy link
Copy Markdown
Contributor

@angelamayxie DOLT

comparing_percentages
100.000000 to 100.000000
version result total
4ff5e11 ok 5937471
version total_tests
4ff5e11 5937471
correctness_percentage
100.0

Copy link
Copy Markdown
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.

LGTM!

@coffeegoddd
Copy link
Copy Markdown
Contributor

@angelamayxie DOLT

comparing_percentages
100.000000 to 100.000000
version result total
b80ef45 ok 5937471
version total_tests
b80ef45 5937471
correctness_percentage
100.0

@angelamayxie angelamayxie merged commit 8074e42 into main Dec 30, 2025
23 checks passed
@angelamayxie angelamayxie deleted the angela/key_conversion branch December 30, 2025 23:27
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.

Lookup Join returns incorrect result due to out-of-range key conversion

3 participants