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

builtins: allow null in enum_* functions #89124

Merged
merged 1 commit into from
Oct 3, 2022

Conversation

HonoreDB
Copy link
Contributor

Our enum functions don't quite have parity with their Postgres equivalents. Even though all they need is the type of their argument, they still evaluate it and error if the result is null. This PR fixes that, not in the ambitious way suggested in #78358 of persisting type info on nulls, but in the more limited way of changing the function implementations to act on their arguments after type resolution but before evaluation.

Informs #78358

Release note (sql change): enum_first, enum_last, and enum_range may now take null arguments as long as their type can be inferred from the expression.
Example: select num_first(null::switch);

Our enum functions don't quite have parity with their
Postgres equivalents. Even though all they need is the
type of their argument, they still evaluate it and error
if the result is null. This PR fixes that, not in the
ambitious way suggested in cockroachdb#78358 of persisting type info
on nulls, but in the more limited way of changing the
function implementations to act on their arguments after
type resolution but before evaluation.

Release note (sql change): enum_first, enum_last, and enum_range may now take null arguments as long as their type can be inferred from the expression.
Example: select num_first(null::switch);
@HonoreDB HonoreDB requested a review from a team September 30, 2022 22:02
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@HonoreDB
Copy link
Contributor Author

This is not needed for anything that I know of other than Postgres compatibility, it was just annoying me today because I was working with enums with really verbose values and wanted to be able to just say "the nth value of this enum". Which there's probably another way to do anyway?

Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

nice fix! are you planning to backport this?

@HonoreDB
Copy link
Contributor Author

HonoreDB commented Oct 3, 2022

bors r=[rafiss]

@HonoreDB
Copy link
Contributor Author

HonoreDB commented Oct 3, 2022

nice fix! are you planning to backport this?

Thanks for the review! As far as I know it isn't worth backporting this right now

@craig
Copy link
Contributor

craig bot commented Oct 3, 2022

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants