fix(plan): allow NULL arguments in GREATEST() and LEAST() - #24628
Merged
XuPeng-SH merged 7 commits intoMay 28, 2026
Conversation
The type checker for GREATEST() and LEAST() required all arguments to have the exact same type OID, which rejected explicit NULL constants (type T_any). This prevented queries like GREATEST(NULL, 1) from working, returning "bad value [ANY BIGINT]" instead of NULL per MySQL behavior. Changes: - Relax leastGreatestCheck() to skip T_any (NULL) arguments when validating that all non-NULL inputs share a common type. - Add leastGreatestParamType() helper so leastFn/greatestFn can find the first non-T_any parameter to determine the dispatch type. - Fix retType to search for the first non-T_any argument, falling back to T_varchar when all inputs are T_any. - Add BVT test cases for NULL argument handling. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
heni02
approved these changes
May 27, 2026
When all arguments are NULL constants (T_any), the dispatch type selector returned T_any which has no matching case in the switch. Fall back to T_varchar so the NULL result is computed correctly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
XuPeng-SH
approved these changes
May 28, 2026
XuPeng-SH
left a comment
XuPeng-SH
left a comment
Contributor
There was a problem hiding this comment.
Reviewed multi-angle: this NULL-handling fix is scoped correctly to the type/dispatch layer. leastGreatestCheck() now ignores T_any only for explicit NULL arguments, dispatch/retType pick the first non-NULL type, and the all-NULL case is routed to an existing varchar path so GREATEST(NULL, NULL) / LEAST(NULL, NULL) return NULL instead of failing. I did not find a correctness blocker in the change.
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
4 participants
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.
What type of PR is this?
Which issue(s) this PR fixes:
issue #24546
What this PR does / why we need it:
The type checker for GREATEST() and LEAST() required all arguments to have the exact same type OID, which rejected explicit NULL constants (type T_any). This prevented queries like
GREATEST(NULL, 1)from working, returning"bad value [ANY BIGINT]"instead of NULL per MySQL behavior.Key changes in
pkg/sql/plan/function/:leastGreatestCheck()now skips T_any (NULL) arguments when validating that all non-NULL inputs share a common type.leastGreatestParamType()helper soleastFn/greatestFncan find the first non-T_any parameter to determine the dispatch type.retTypecallbacks now search for the first non-T_any argument, falling back toT_varcharwhen all inputs are T_any.Special notes for your reviewer:
The base implementation of GREATEST() and LEAST() was already merged in #22838. This PR fixes a gap where explicit NULL values were rejected during type checking. The NULL propagation behavior (return NULL if any arg is NULL) was already correct in the execution functions; the fix is purely in the type checking layer.