cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.1k stars 3.81k forks source link

sql: truncate floats to FLOAT4 correctly #73743

Open mgartner opened 2 years ago

mgartner commented 2 years ago

When inserting a float into a FLOAT4 column, or casting a value to a FLOAT4, CRDB does not correctly truncate the precision of the float to fit within a FLOAT4. This is present in v21.2.2, so it's not a regression from the assignment cast changes that were merged after v21.2, but it is related to assignment casts.

In Postgres:

marcus=# CREATE TABLE t (
  f4 FLOAT4,
  f8 FLOAT8
);
CREATE TABLE

marcus=# INSERT INTO t VALUES (18754999.99, 18754999.99);
INSERT 0 1

marcus=# SELECT * FROM t;
     f4     |     f8
------------+-------------
 1.8755e+07 | 18754999.99
(1 row)

marcus=# SELECT f8::FLOAT4 FROM t;
     f8
------------
 1.8755e+07
(1 row)

In CRDB (using logic test because the demo client will truncate floats):

statement ok
CREATE TABLE t (
  f4 FLOAT4,
  f8 FLOAT8
)

statement ok
INSERT INTO t VALUES (18754999.99, 18754999.99)

# The value of f4 should be 1.8755e+07.
query RR colnames
SELECT * FROM t
----
f4               f8
1.875499999e+07  1.875499999e+07

# The result should be 1.8755e+07.
query R
SELECT f8::FLOAT4 FROM t
----
1.875499999e+07

Jira issue: CRDB-11718

mgartner commented 2 years ago

The issue can be observed with explicit casts in the demo. In the example below, the result value should be 18755000.

defaultdb> SELECT 18754999.99::FLOAT8::FLOAT4::FLOAT8;
      float8
-------------------
  1.875499999e+07
(1 row)
otan commented 2 years ago

root cause is float to float casts do nothing:

https://github.com/cockroachdb/cockroach/blob/c089297b4915fd58b03af2323e0442ecfc042c2f/pkg/sql/sem/tree/cast.go#L1878-L1879

for int family, it is done here...

https://github.com/cockroachdb/cockroach/blob/c089297b4915fd58b03af2323e0442ecfc042c2f/pkg/sql/sem/tree/cast.go#L1516

otan commented 2 years ago

float4 is actually handled pretty badly.

parsing example:

root@127.0.0.1:26257/defaultdb> select '4e+38'::float4;
   float4
------------
  Infinity
(1 row)

vs psql:

otan=# select '4e+38'::float4;
ERROR:  "4e+38" is out of range for type real
LINE 1: select '4e+38'::float4;
otan commented 2 years ago

urgh, this is already committed into the DB for some people as well. would need to test whether any fix breaks those cases...

otan commented 2 years ago

in fact, since we store Float4 as Float8, all arithmetic is done using Float8 so even more stuff is probably wrong?

mgartner commented 2 years ago

in fact, since we store Float4 as Float8, all arithmetic is done using Float8 so even more stuff is probably wrong?

Yikes, I think you're probably correct about that. Looks like this won't be a quick win...

rafiss commented 1 year ago

The root problem here seems to be https://github.com/cockroachdb/cockroach/issues/48613

mgartner commented 1 year ago

It looks like this was partially fixed by #82022. See: https://github.com/cockroachdb/cockroach/commit/4d16dbfb078c071aeeb7f964eb075f443d0aa091#diff-c49e1bdbc0004860cadef9d5e1e4d3e2c18b89a254ad9f4885e6c929ff2a5e43R172-R173

But it's not clear to me how that change fixed these tests and the example test in the description above. Maybe @rafiss has an idea?

Regardless, I think this is still an issue as seen here: https://github.com/cockroachdb/cockroach/commit/4d16dbfb078c071aeeb7f964eb075f443d0aa091#diff-c49e1bdbc0004860cadef9d5e1e4d3e2c18b89a254ad9f4885e6c929ff2a5e43R1405-R1411

rafiss commented 1 year ago

Another related issue: https://github.com/cockroachdb/cockroach/issues/84326