Skip to content

Add cxjs_nullif() - #131

Merged
gbeeley merged 3 commits into
masterfrom
add-cxjs_nullif
Aug 31, 2026
Merged

Add cxjs_nullif()#131
gbeeley merged 3 commits into
masterfrom
add-cxjs_nullif

Conversation

@Lightning11wins

Copy link
Copy Markdown
Contributor

This PR will add the cxjs_nullif() on the client, which runs with the same logic as the server-side version.

@Lightning11wins Lightning11wins self-assigned this Jul 21, 2026
@Lightning11wins Lightning11wins added ai-review Request AI review for PRs. size: trivial Easy to review, probably ~100 lines or fewer. labels Jul 21, 2026
@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds client-side support for cxjs_nullif() in the rendering helpers. The main change is:

  • A new two-argument helper using coercive equality to mirror server-side comparison.

Confidence Score: 4/5

The null second-operand path needs a fix before merging.

  • cxjs_nullif(nonNullValue, null) returns null instead of preserving the first value.
  • This can make client and server expression results differ.

centrallix-os/sys/js/ht_render.js

Important Files Changed

Filename Overview
centrallix-os/sys/js/ht_render.js Adds cxjs_nullif(), but a null second operand incorrectly nullifies a non-null first operand.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
centrallix-os/sys/js/ht_render.js:324-325
**Null Second Operand Drops Value**

When `a` is non-null and `b` is null, this branch returns null instead of `a`. For example, `cxjs_nullif('value', null)` produces null, so client expressions can disagree with the server-side `NULLIF` result.

```suggestion
    if (a == null || a == b) return null;
    return a;
```

Reviews (1): Last reviewed commit: "Add cxjs_nullif()." | Re-trigger Greptile

Comment thread centrallix-os/sys/js/ht_render.js Outdated
@Lightning11wins

Copy link
Copy Markdown
Contributor Author

This PR is cleared for human review.

@gbeeley

gbeeley commented Aug 21, 2026

Copy link
Copy Markdown
Member

Let's fix the behavior on the server and client to match other SQLs on the case of arg1 is null.

@Lightning11wins

Lightning11wins commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Let's fix the behavior on the server and client to match other SQLs on the case of arg1 is null.

@gbeeley In the case where the first argument is null, both the Centrallix server and client nullif implementations return null. As far as I know, PG, MySQL, SQLite, and MSSQL follow the same pattern.

(Claude says that Oracle SQL rejects the literal value null at parse-time but follows this pattern and returns null if given a column value or expression that evaluates to null.)

@gbeeley

gbeeley commented Aug 31, 2026

Copy link
Copy Markdown
Member

Oops, typo, let's fix the behavior on both server and client to match other SQLs, e.g. if arg1 is not null and arg2 is null, preserve arg1 rather than returning null.

@Lightning11wins
Lightning11wins removed the request for review from nboard August 31, 2026 19:09
@Lightning11wins

Copy link
Copy Markdown
Contributor Author

@gbeeley Ahh, understood. Updated.

@gbeeley gbeeley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - thanks

@gbeeley
gbeeley merged commit 620c30f into master Aug 31, 2026
1 check passed
@Lightning11wins
Lightning11wins deleted the add-cxjs_nullif branch September 2, 2026 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request AI review for PRs. size: trivial Easy to review, probably ~100 lines or fewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants