fix sqrt(-1.0::float8) should error, not return NaN#22308
Merged
Conversation
Co-authored-by: Copilot <copilot@github.com>
Member
Author
|
@Dandandan Could you help me take a look? |
kosiew
approved these changes
May 23, 2026
kosiew
left a comment
Contributor
There was a problem hiding this comment.
@xiedeyantu
Looks 👍 to me
Member
Author
Thanks! |
Dandandan
approved these changes
May 23, 2026
Member
Author
|
@Dandandan CI appears to be malfunctioning. |
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.
Which issue does this PR close?
sqrt(-1.0::float8)should error, not return NaN #22260.Rationale for this change
DataFusion previously returned
NaNforsqrton negative floating-point inputs, for examplesqrt((-1.0)::float8). This differs from PostgreSQL semantics, which raise an error for square root of a negative number.This change makes
sqrtreturn an execution error for out-of-domain negative inputs so its behavior is closer to PostgreSQL and avoids silently producingNaNfor invalid inputs.What changes are included in this PR?
sqrtto use a named validator helper instead of inline predicate and error-string arguments.sqrtso negative inputs now raisecannot take square root of a negative number.sqrt:Are these changes tested?
Yes.
The change is covered by existing SQL logic tests and targeted validation runs:
cargo test -p datafusion-functions sqrtcargo test -p datafusion-sqllogictest --test sqllogictests scalarAre there any user-facing changes?
Yes.
sqrtnow raises an execution error for negative inputs instead of returningNaN. This changes user-visible query behavior to better align with PostgreSQL semantics.