fix(orm): use IS operator for null comparisons in filters

[ymc9]

Summary

• Fixed { fieldName: { not: null } } filter returning empty results instead of non-null records
• Root cause: buildLiteralFilter() used SQL = operator for null comparisons (column = NULL is always unknown in SQL), now uses IS operator (column IS NULL)
• Added regression test and filter e2e test coverage for { not: null } on string and numeric fields

Closes #2472

Test plan

• Regression test issue-2472.test.ts passes (DateTime and String nullable fields)
• Filter e2e tests pass with new { not: null } cases for string (name) and numeric (age) fields
• All existing find and filter tests continue to pass

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Enhanced how null and undefined values are handled in filter queries. The not: null shorthand filter now correctly identifies non-null records across all field types, including strings, numbers, and dates.

• Tests

  • Added regression and end-to-end test coverage validating null filtering behavior across different field types.

[claude]

Claude finished @ymc9's task —— View job

---

PR Review: fix(orm): use IS operator for null comparisons in filters

Review Checklist

• Analyze the core dialect change in base-dialect.ts
• Review regression test coverage (issue-2472.test.ts)
• Examine e2e filter test additions
• Check for potential edge cases and bugs
• Assess performance implications
• Verify security considerations
• Provide comprehensive feedback

Summary

This PR fixes a critical SQL correctness issue where { fieldName: { not: null } } filters returned empty results instead of records with non-null values. The fix properly addresses the root cause and includes comprehensive test coverage.

Code Quality & Technical Review

✅ Core Fix - Excellent

The change in base-dialect.ts:763-775 is spot-on:

• Root Cause: buildLiteralFilter() used = operator for null comparisons (column = NULL is always unknown/false in SQL)
• Solution: buildValueFilter() correctly uses IS operator (column IS NULL) for null comparisons
• Implementation: Clean separation of concerns - explicit null handling vs value transformation

✅ Test Coverage - Comprehensive

• Regression Test: issue-2472.test.ts covers both DateTime and String nullable fields with realistic scenarios
• E2E Coverage: Existing filter tests now include { not: null } cases for string (name) and numeric (age) fields
• Edge Cases: Tests cover multiple data types and realistic use cases

✅ SQL Correctness - Critical Fix

This addresses a fundamental SQL semantic issue:

-- Before (incorrect): WHERE column = NULL  → always false/unknown
-- After (correct):   WHERE column IS NULL → proper null comparison

Performance & Security

✅ Performance - Neutral/Positive

• IS NULL vs = NULL has identical or better performance on all SQL databases
• No impact on query plan generation
• Standard SQL operation with optimal database handling

✅ Security - Safe

• Changes only affect null comparison operators
• No SQL injection vectors introduced
• Uses parameterized queries via Kysely's expression builder

Edge Cases & Considerations

✅ Comprehensive Null Handling

The new buildValueFilter() method handles all null scenarios correctly:

• undefined → no-op filter (always true)
• null → IS NULL comparison
• Non-null values → standard = comparison with proper type transformation

✅ Backwards Compatibility

• Only affects the broken { not: null } case
• All existing valid filters remain unchanged
• No breaking changes to API

Minor Observations

1. Method Naming: buildValueFilter() is more descriptive than buildLiteralFilter() ✓
2. Code Organization: The refactor improves separation between null handling and value transformation ✓
3. Type Safety: Maintains strong typing throughout the change ✓

Verdict: Approved ✅

This is a high-quality fix that:

• ✅ Addresses the exact root cause with minimal, targeted changes
• ✅ Includes comprehensive test coverage for multiple data types
• ✅ Maintains backwards compatibility while fixing broken functionality
• ✅ Uses correct SQL semantics following database standards
• ✅ Has no negative performance or security implications

The PR successfully resolves issue #2472 and significantly improves the reliability of null filtering operations.

---

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ea5064ab-dc39-4147-9240-b09bbb13c78c

📥 Commits

Reviewing files that changed from the base of the PR and between d3ab3a6 and 6d00b43.

📒 Files selected for processing (3)

• packages/orm/src/client/crud/dialects/base-dialect.ts
• tests/e2e/orm/client-api/filter.test.ts
• tests/regression/test/issue-2472.test.ts

---

📝 Walkthrough

Walkthrough

This PR fixes the not: null filter functionality by refactoring JSON equality handling in the base dialect. The change replaces literal-based filtering with value-based filtering, introducing proper handling for undefined and null values. Includes corresponding e2e and regression tests.

Changes

Cohort / File(s)	Summary
Core Dialect Logic packages/orm/src/client/crud/dialects/base-dialect.ts	Refactored buildJsonEqualityFilter to use new buildValueFilter helper instead of buildLiteralFilter. Handles undefined (returns no-op true), null (IS NULL check), and regular values (equality check) with distinct semantics. Updated buildStandardFilter to route through value-based filtering.
Test Coverage tests/e2e/orm/client-api/filter.test.ts, tests/regression/test/issue-2472.test.ts	Added e2e tests for not: null shorthand on string and numeric fields, plus new regression test suite validating not: null filtering on DateTime and String nullable fields with multiple schema contexts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A filter fix hops through the code,
Where not: null now finds its road!
Empty arrays fade away,
Non-null values see the day! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing null comparison operators in filter SQL generation from = to IS operator.
Linked Issues check	✅ Passed	The PR fully addresses issue #2472 by fixing SQL generation for null comparisons using IS operator instead of =, adding regression tests for DateTime and String fields, and extending filter e2e tests with not: null cases.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the not: null filter issue through dialect refactoring and comprehensive test coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/not-null-filter-2472

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}