duckdb / duckdb/pg_duckdb

NUMERIC NaN read from a Postgres table is silently converted to 0 when executed by DuckDB

Open
#1,069 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
3.2k
Forks
204
PR merge metrics
No merged PRs in 30d

Description

### What happens?

A `NUMERIC` column holding `NaN` is silently converted to `0` when the query is executed by DuckDB, so the result disagrees with Postgres with no warning. The same happens to `NaN`, `Infinity` and `-Infinity` in an unconstrained `NUMERIC` when `duckdb.convert_unsupported_numeric_to_double` is on. (The `real` and `double precision` types are unaffected — they never go through this conversion.)

Unlike #1039, which is about a NaN *constant* and fails loudly, this one is silent.

### To Reproduce

```sql
CREATE TABLE special_numerics (id int, dec numeric(10,2), unbound numeric);
INSERT INTO special_numerics VALUES
(1, 'NaN', 'NaN'), (2, 10.00, 10), (3, NULL, 'Infinity'), (4, NULL, '-Infinity');

SET duckdb.force_execution = false;
SELECT sum(dec), avg(dec), min(dec) FROM special_numerics;
-- sum | avg | min <-- Postgres
-- -----+-----+-------
-- NaN | NaN | 10.00

SET duckdb.force_execution = true;
SELECT sum(dec), avg(dec), min(dec) FROM special_numerics;
-- sum | avg | min <-- DuckDB: all three wrong, no warning
-- -------+-----+------
-- 10.00 | 5 | 0.00

SET duckdb.convert_unsupported_numeric_to_double = true;
SELECT id, unbound FROM special_numerics ORDER BY id;
-- id | unbound expected: NaN, 10, Infinity, -Infinity
-- ----+---------
-- 1 | 0
-- 2 | 10
-- 3 | 0
-- 4 | 0
```

### Root cause

Postgres stores `NaN` / `±Infinity` as "special" numerics, which have no digits. `FromNumeric()` (`src/pgduckdb_types.cpp`) does not check `NUMERIC_IS_SPECIAL()`, and `ConvertDecimal()` then turns anything with no digits into `0`:

```cpp
if (numeric.ndigits == 0) {
return 0;
}
```

The vendored header the code relies on warns about exactly this (`include/pgduckdb/vendor/pg_numeric_c.hpp`, above `NUMERIC_SIGN`):

> Note that we don't trouble to ensure that dscale and weight read as zero for an infinity; however, that doesn't matter since we never convert "special" numerics to NumericVar form.

There is no NaN or Infinity coverage in `test/regression/` or `test/pycheck/`, which is why CI stays green.

### Proposed fix

Opened as #1070.

The `NUMERIC` → `DOUBLE` conversion is straightforward: all three values map onto their IEEE 754 counterparts, so that path round-trips correctly. A DuckDB `DECIMAL` however has no representation for them at all, so the patch raises a conversion error there — the same thing #1039 already does for constants — instead of returning a wrong number.

I also considered rejecting `numeric(p,s)` at planning time so that such queries would fall back to Postgres transparently, but that would cost DuckDB acceleration for every `numeric` column whether or not it ever holds a special value. Happy to reconsider if you disagree.

### OS:

Linux (Debian bookworm, arm64)

### pg_duckdb Version (if built from source use commit hash):

ee38d3b

### Postgres Version (if built from source use commit hash):

Reproduced on 14.24 and 18.6

### Hardware:

Apple Silicon, 8 cores, 32 GB RAM (8 CPUs / 8 GB allocated to Docker)

### Full Name:

Liangrun Da

### Affiliation:

Alibaba Cloud

### What is the latest build you tested with? If possible, we recommend testing with the latest nightly build.

I have tested with a source build

### Did you include all relevant data sets for reproducing the issue?

Not applicable - the reproduction does not require a data set

### Did you include all code required to reproduce the issue?

- [X] Yes, I have

### Did you include all relevant configuration (e.g., CPU architecture, Linux distribution) to reproduce the issue?

- [X] Yes, I have

Contributor guide

Open the contributing guide

Research direction

Start in src/pgduckdb_types.cpp with FromNumeric() and follow ConvertDecimal(); review the special-value warning in include/pgduckdb/vendor/pg_numeric_c.hpp. Add regression coverage under test/regression/ or test/pycheck/ for NaN and infinities, then verify DOUBLE conversion preserves them and DECIMAL conversion reports an error rather than returning zero.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, postgresql
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.