Skip to content

fix: ISNULL() should fill in the replacement for NULL instead of behaving like NULLIF() - #2561

Closed
harsh-thakkar7 wants to merge 1 commit into
AlaSQL:developfrom
harsh-thakkar7:fix/isnull-replacement-semantics
Closed

harsh-thakkar7 wants to merge 1 commit into
AlaSQL:developfrom
harsh-thakkar7:fix/isnull-replacement-semantics

Conversation

@harsh-thakkar7

Copy link
Copy Markdown

Description

ISNULL() is currently aliased to NULLIF():

// src/55functions.js
stdlib.ISNULL = stdlib.NULLIF = function (a, b) {
	return "(" + a + "==" + b + "?undefined:" + a + ")";
};

In T-SQL (the dialect ISNULL comes from), ISNULL(check, replacement) returns the replacement when the first argument is NULL, and the first argument otherwise. Aliasing it to NULLIF makes ISNULL(NULL, 9) evaluate to NULL/undefined instead of 9:

alasql("SELECT ISNULL(null, 9) AS v"); // [{ }]  -- should be [{"v":9}]

Changes

  • src/55functions.js: give ISNULL() its own implementation with the same codegen pattern as the adjacent IFNULL() (undefined/null check → return replacement; otherwise keep the value). NULLIF() keeps its existing behavior.
  • test/test2227.js: 7 tests. C) ISNULL(9, 9) returns 9 is the regression discriminator — under the old NULLIF aliasing it returned undefined. Also guards that NULLIF() and the IS NULL expression are untouched.

Tests

Full suite passes (0 failures).

alasql("SELECT ISNULL(null, 9) AS v"); // [{"v":9}]
alasql("SELECT NULLIF(5, 5) AS v");     // [{ }] -- unchanged

…ving like NULLIF()

Signed-off-by: Harsh Thakkar <harsh-thakkar7@users.noreply.github.com>
@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