sql: preserve common type for homogeneous builtins - #174720
Open
Alignyx wants to merge 1 commit into
Open
Conversation
Homogeneous overload resolution can select a wider common type while leaving the first argument with a narrower, equivalent type. Since least and greatest derive their return type from that argument, a wider selected value can then be truncated by the result slot. Retype only the first non-NULL homogeneous argument to the already checked common type. Add type-checking and SQL regressions for mixed integer widths, including commuted, promoted, NULL, and greatest variants. Fixes cockroachdb#159153 Release note (bug fix): Fixed incorrect truncation in nested least and greatest calls over mixed-width integer arguments.
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #159153.
Root cause
Homogeneous overload resolution selected
INT4as the common type for mixed-width integer arguments, but equivalent integer types were allowed to keep their original widths.leastandgreatestderive their return type from the first non-NULL argument, so a narrowINT2first argument made the result slotINT2even when evaluation selected anINT4value. The wider value was then truncated; in the reported case-1347373370became-18746.How I tracked it down
I reproduced the nested
leastresult and compared it with commuting the arguments, explicitly promoting the narrow argument, and an equivalentCASEexpression. Those controls agreed on the full-width result, while the flat mixed-widthleastcall remained correct. The plans differed only in scalar rendering. A focused overload-resolution test then showed the violated invariant directly: common-type selection returnedINT4, but the first return-determining expression still resolved asINT2. That localized the bug to theHomogeneousTypeoverload path rather than comparison or vectorized evaluation.Fix
After homogeneous arguments have been validated and their common type selected, overload resolution now retypes only the first non-NULL argument to that common type when necessary. This keeps the change scoped to builtins that use
HomogeneousTypeandFirstNonNullReturnType, while preserving the existing common-type checks and the types of the remaining arguments.Test coverage
INT4column, asserting that the homogeneous arguments and return-determining expression resolve asINT4.least, commuted arguments, explicit promotion, a NULL outer argument, the correspondinggreatestcase, and a flat mixed-width control.