Skip to content

Simplify nvl2 implementation & fix coercion behaviour - #18127

Closed
Jefffrey wants to merge 1 commit into
apache:mainfrom
Jefffrey:refactor-nvl2
Closed

Jefffrey wants to merge 1 commit into
apache:mainfrom
Jefffrey:refactor-nvl2

Conversation

@Jefffrey

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Refactor the implementation to simplify it, and also fix which types we coerce.

What changes are included in this PR?

When coercing input types, we ignore the 1st argument (it is only used to calculate if return 2nd or 3rd argument so no need to consider it for coercion). Also simplify the actual implementation.

Are these changes tested?

Existing tests.

Are there any user-facing changes?

No.

@github-actions github-actions Bot added the functions Changes to functions implementation label Oct 17, 2025
@Jefffrey

Copy link
Copy Markdown
Contributor Author

Maybe this becomes unnecessary if it'll get rewritten by #17983 anyway 🤔

@Jefffrey
Jefffrey marked this pull request as ready for review October 17, 2025 13:30
@pepijnve

pepijnve commented Oct 17, 2025 •

Copy link
Copy Markdown
Contributor

I was planning on doing the same simplification as in #17982 where nvl2(x, y, z) would get simplified to case when x is not null then y else z end. The rationale is to try to make the various lazy, conditional logic variants (nvl, nvl2, coalesce) all aliases of CaseExpr so that we only have to optimise it once.

@Jefffrey

Copy link
Copy Markdown
Contributor Author

I was planning on doing the same simplification as in #17982 where nvl2(x, y, z) would get simplified to case when x is not null then y else z end. The rationale is to try to make the various lazy, conditional logic variants (nvl, nvl2, coalesce) all aliases of CaseExpr so that we only have to optimise it once.

Sounds good, I'll close this PR and close the refactor issue as unplanned

Also fyi @kosiew since I see you self-assigned #17983

@Jefffrey Jefffrey closed this Oct 17, 2025
@Jefffrey
Jefffrey deleted the refactor-nvl2 branch October 17, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Minor issue with nvl2_func

2 participants