Skip to content

fix: INSERT without a column list stores NaN instead of NULL in numeric columns - #2563

Closed
harsh-thakkar7 wants to merge 1 commit into
AlaSQL:developfrom
harsh-thakkar7:fix/null-insert-into-typed-column
Closed

harsh-thakkar7 wants to merge 1 commit into
AlaSQL:developfrom
harsh-thakkar7:fix/null-insert-into-typed-column

Conversation

@harsh-thakkar7

Copy link
Copy Markdown

Problem

INSERT INTO t VALUES (NULL) against a table with a numeric column (INT/FLOAT/NUMBER/MONEY) stores NaN instead of NULL when no column list is given:

CREATE TABLE t (v INT);
INSERT INTO t VALUES (NULL);
SELECT * FROM t WHERE v IS NULL;      -- returns 0 rows (!!!)
SELECT * FROM t WHERE v IS NOT NULL;  -- returns the NULL row

Standard functions like IS NULL and COALESCE then behave incorrectly on those rows.

The same statement with a column list (INSERT INTO t (v) VALUES (NULL)) works correctly and stores undefined (NULL).

Root cause

In the no-column-list branch of the INSERT compiler (src/70insert.js), numeric columns are compiled as:

q += '+' + values[idx].toJS();

A NULL value compiles to undefined, and +undefined yields NaN. Every other path in the same function already guards this:

q += '(x=' + values[idx].toJS() + ',x==undefined?undefined:+x)';

The fix applies that identical guard to the no-column-list branch, so NULL is stored as undefined and retains the existing numeric coercion for real values.

Tests

test/test2229.js:

  • INT column (no column list): NULL matches WHERE v IS NULL, numbers are still coerced to numbers
  • FLOAT column (no column list): same behavior
  • with-column-list path is unchanged
  • non-numeric columns are unchanged

Full suite: 2242 pass / 0 fail / 213 skip.

@harsh-thakkar7

Copy link
Copy Markdown
Author

Closing this PR — it's been open with no maintainer movement for several days, and I don't want to leave noise in the queue. If the change is still wanted, I'm happy to reopen or resubmit with any requested adjustments. Thanks for the repo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant