sql: add bounds checking for trig functions#168717
sql: add bounds checking for trig functions#168717trunk-io[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
😎 Merged successfully - details. |
|
This change is |
fqazi
left a comment
fqazi
left a comment
There was a problem hiding this comment.
@fqazi reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on bghal).
rafiss
left a comment
rafiss
left a comment
There was a problem hiding this comment.
nice find! acosh and atanh have the identical NaN-instead-of-error bug in CRDB and postgres rejects them too. can we fix them now?
@rafiss made 2 comments.
Reviewable status:
-- commits line 11 at r1:
for this to get categorized properly, it should say Release note (bug fix)
bghal
force-pushed
the
sql-trig-input
branch
3 times, most recently
from
fa46206 to
4db389e
Compare
bghal
left a comment
bghal
left a comment
There was a problem hiding this comment.
Done.
@bghal made 2 comments.
Reviewable status:
Previously, rafiss (Rafi Shamim) wrote…
for this to get categorized properly, it should say
Release note (bug fix)
Done.
|
Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link) |
There was a problem hiding this comment.
@rafiss made 1 comment.
Reviewable status:
pkg/sql/sem/eval/testdata/eval/trig line 172 at r2 (raw file):
eval atanh(1)
this is overly strict. PG allows inputs in the range of [-1, 1] (inclusive bounds)
postgres=# select atanh(1.0);
atanh
----------
Infinity
(1 row)
postgres=# select atanh(-1.0);
atanh
-----------
-Infinity
(1 row)
please fix this and double check the other functions validation. tip: you don't need to do this manually. ask claude to look at the postgres source code. (you can clone the repo locally onto your own machine so it doesn't have to do slow network operations.)
bghal
force-pushed
the
sql-trig-input
branch
2 times, most recently
from
8f92217 to
cb2771e
Compare
bghal
changed the title
bghal
force-pushed
the
sql-trig-input
branch
3 times, most recently
from
b8d1752 to
e17fe53
Compare
The previous behavior of returning NaN was inconsistent with PG and may have caused downstream errors. The forward trig functions weren't rejecting infinity as input which is also inconsistent with PG. Epic: none Fixes: cockroachdb#168595 Release note (bug fix): Trig functions passed out-of-range input now return errors consistent with PostgreSQL.
|
Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link) |
4 participants
The previous behavior of returning NaN was
inconsistent with PG and may have caused
downstream errors. The forward trig functions
weren't rejecting infinity as input which is also
inconsistent with PG.
Epic: none
Fixes: #168595
Release note (bug fix): Trig functions passed out-of-range input now
return errors consistent with PostgreSQL.