Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Imprecise type conversion from UBIGINT to DOUBLE PRECISION #246

Closed
2 tasks done
Mickael-van-der-Beek opened this issue Aug 12, 2024 · 1 comment · Fixed by #256
Closed
2 tasks done

Imprecise type conversion from UBIGINT to DOUBLE PRECISION #246

Mickael-van-der-Beek opened this issue Aug 12, 2024 · 1 comment · Fixed by #256

Comments

@Mickael-van-der-Beek
Copy link

Mickael-van-der-Beek commented Aug 12, 2024

What happens?

DuckDB supports unsigned 64 bit integers (UBIGINT).

PostgreSQL on the other hand only supports signed 64 bit integers (BIGINT).

As such, I would expect DuckDB to fail to insert rows containing UBIGINT data to PostgreSQL.

I confirmed that this is the case when trying to export data to a file using the POSGRES_BINARY format.

However, when inserting data directly from DuckDB to PostgreSQL there is no error and DuckDB decides to cast the UBIGINT value to DOUBLE PRECISION automatically.

I understand that it's impossible to store UBIGINT values in PostgreSQL but I would expect the query to simply error out in this case.

This casting behaviour corrupted one of our database tables silently.

To Reproduce

In DuckDB run the following commands:

INSTALL 'postgres';
LOAD 'postgres';

ATTACH
    'host=myhost.com port=5432 dbname=my_database user=root password=toor'
 AS
    my_postgresql
    (
      TYPE postgres
    )
 ;

Testing the POSTGRES_BINARY behaviour:

COPY (
  SELECT
    7540279650942975244::UBIGINT
      AS id
) TO
  '/some/folder/my-export.bin'
  (
    FORMAT POSTGRES_BINARY
  )
;

This query throws the error: Not implemented Error: Type "UBIGINT" is not supported for Postgres binary copy

Testing the copy-to-PostgreSQL behaviour:

CREATE UNLOGGED TABLE
    my_postgresql.tmp_table
      AS
      (
        SELECT
          1394265502879208450::UBIGINT
            AS id
      )
 ;

And then if we look at the table definition in PostgreSQL, we see that:

=> \d+ tmp_table;
                                            Table "public.tmp_table"
 Column |       Type       | Collation | Nullable | Default | Storage | Compression | Stats target | Description 
--------+------------------+-----------+----------+---------+---------+-------------+--------------+-------------
 id     | double precision |           |          |         | plain   |             |              | 
Access method: heap

So an invalid type conversion took place from UBIGINT to Double Precision.

Which creates annoying behaviour like this:

SELECT * FROM tmp_table WHERE id::BIGINT = 1394265502879208450::BIGINT;

Which returns 0 rows as a result because the Double Precision to BIGINT cast modifies the literal value due to the dynamic precision of the type:

=> SELECT id::BIGINT FROM tmp_table;
         id          
---------------------
 1394265502879208448
(1 row)

1394265502879208448 being a close but different value than 1394265502879208450.

OS:

OSX

PostgreSQL Version:

16.3

DuckDB Version:

1.0.0

DuckDB Client:

CLI & Node.js

Full Name:

Mickael van der Beek

Affiliation:

Alliance Gravity

Have you tried this on the latest main branch?

  • I agree

Have you tried the steps to reproduce? Do they include all relevant data and configuration? Does the issue you report still appear there?

  • I agree
@Mickael-van-der-Beek Mickael-van-der-Beek changed the title Incorrect type conversion from UBIGINT to Double Precision Incorrect type conversion from UBIGINT to DOUBLE PRECISION Aug 12, 2024
@Mytherin Mytherin changed the title Incorrect type conversion from UBIGINT to DOUBLE PRECISION Imprecise type conversion from UBIGINT to DOUBLE PRECISION Sep 3, 2024
Mytherin added a commit that referenced this issue Sep 3, 2024
@Mytherin
Copy link
Contributor

Mytherin commented Sep 3, 2024

Thanks for the report! I've pushed a fix in #256 that converts UBIGINT to a DECIMAL(20,0) instead

Mytherin added a commit that referenced this issue Sep 4, 2024
Fix  #246 - convert ubigint to `decimal(20,0)` instead of to double to avoid loss of precision
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants