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: jdbc DECIMAL wrong precision and scale #15748

Closed arduanov closed 7 years ago

arduanov commented 7 years ago

I use IDEA PHPStorm for test, but other java libs thru jdbc works exactly

CREATE TABLE test
(
    dec DECIMAL(2,1)
);

insert into test VALUES (1);

SELECT * from test;

Result

1.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

and jdbc generated DDL


-- auto-generated definition
create table test
(
    dec numeric(131089)
)
petermattis commented 7 years ago

@mjibson Can you take a look? I doubt this has anything to do with our decimal code, though.

maddyblue commented 7 years ago

@arduanov I need some more information. In that first code block, why is there no "CREATE TABLE" string in it? When you say "jdbc generated DDL", how did the jdbc generate that DDL? I don't use either PHPStorm or jdbc so I need some pointers.

robert-s-lee commented 7 years ago

Attached is an Apache Jmeter adoption of https://www.cockroachlabs.com/docs/build-a-java-app-with-cockroachdb.html example. The expected balance is always 100; however, the balance is coming back as 100.00000.... Toggle Cockroach or Postgres JDBC connection to see the different in behavior between the databases.

Below are the steps in the JMeter test plan

setup thread

CREATE TABLE IF NOT EXISTS accounts (id INT PRIMARY KEY, balance INT)

insert INTO accounts (id, balance) VALUES (1, 40), (2, 60) ON CONFLICT (id) DO UPDATE SET balance = excluded.balance

txn thread

UPDATE accounts SET balance = balance - 10 where id = 1 UPDATE accounts SET balance = balance + 10 where id = 2

balance thread SELECT sum(balance) FROM accounts where id in (1,2)

acctbal.jmx.zip

cuongdo commented 7 years ago

@jordanlewis could you take a look?

jordanlewis commented 7 years ago

@robert-s-lee, nice find! That's likely a different incompatibility than the one mentioned in the OP, though. The problem here seems to be a type mismatch in the output of SUM. Also, it looks like this only occurs over pgwire, as I can't repro via the sql shell, so it's probably an issue with our type hinting.

Can you open a new issue to track this? Thanks!

I agree with @mjibson that we need some more detail to solve the original issue.

arduanov commented 7 years ago

Jmeter test decimal.jmx.zip

****** received  : 1.0[[[0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000...]]]

****** comparison: 1.0
jordanlewis commented 7 years ago

Thanks @arduanov! I see now that there are several related problems with our DECIMAL type.

We don't seem to respect the input precision and scale when formatting decimals at all. Here are a few examples of the inconsistencies. I'm not sure if fixing all of these will also fix the issue you're seeing, but it seems that regardless we're promising something about decimal semantics that we're not delivering.

In CRDB:

root@:26257/test> SELECT 1.00::decimal(6,4);
+--------------------+
| 1.00::DECIMAL(6,4) |
+--------------------+
|               1.00 |
+--------------------+
(1 row)
root@:26257/test> SELECT 101.00::decimal(6,4);
+----------------------+
| 101.00::DECIMAL(6,4) |
+----------------------+
|               101.00 |
+----------------------+
(1 row)
root@:26257/test> SELECT 101.00::decimal(4,6);
+----------------------+
| 101.00::DECIMAL(4,6) |
+----------------------+
|               101.00 |
+----------------------+
(1 row)

In Postgres:

jordan=# select 1.00::decimal(6,4);
 numeric
---------
  1.0000
(1 row)

jordan=# select 101.0::decimal(6,4);
ERROR:  numeric field overflow
DETAIL:  A field with precision 6, scale 4 must round to an absolute value less than 10^2.
jordan=# select 101.0::decimal(4,6);
ERROR:  NUMERIC scale 6 must be between 0 and precision 4
LINE 1: select 101.0::decimal(4,6);
jordanlewis commented 7 years ago

@mjibson could you please triage this? I assume our decimal library can handle these issues and that we just need to augment parsing and printing.

maddyblue commented 7 years ago

Yes, I'll work on this.

maddyblue commented 7 years ago

I can reproduce the issues with JMeter. The problem is here:

https://github.com/cockroachdb/cockroach/blob/47d79cc80dd11195cd3d401e949684edb0227190/pkg/sql/pgwire/v3.go#L1083

Postgres, for this specific type, returns -1 while we return 0. If crdb is modified to return a -1 there, then the test passes. (See http://stackoverflow.com/questions/3350148/where-are-numeric-precision-and-scale-for-a-field-found-in-the-pg-catalog-tables for some details about how this field can encode precision and scale for decimals.) I think this is an issue for the ORM team.

cuongdo commented 7 years ago

@justinj this might be a good segue into this quarter's Java work