fix(sql): render negated filter operators null-inclusively - #758
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughNegated SQL filters now include ChangesNegated filter complement semantics
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant FilterInput
participant TypeormAdapter
participant FiltersVisitor
participant Database
participant compileFilters
FilterInput->>TypeormAdapter: submit positive and negated filters
TypeormAdapter->>FiltersVisitor: compile SQL predicates
FiltersVisitor->>Database: execute NULL-inclusive conditions
Database-->>TypeormAdapter: return matching record IDs
FilterInput->>compileFilters: evaluate the same filters in memory
compileFilters-->>FilterInput: return expected record IDs
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Negated filter operators (ne, nin, notContains, notStartsWith, notEndsWith) rendered as bare SQL negations, which drop NULL rows under three-valued logic. They now render null-inclusively ((field <> ? or field is null)), making every negated operator the exact logical complement of its positive twin — the semantics @rapiq/memory already implements. The TypeORM adapter inherits the fix through the shared visitor; live-DB specs pin the inheritance and a cross-adapter spec replays the complement-law matrix via both @rapiq/memory and the TypeORM adapter, asserting identical record sets. Closes #752
tada5hi
force-pushed
the
fix/752-sql-complement-law
branch
from
July 13, 2026 13:11
653e07e to
cb1a28f
Compare
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/sql/test/unit/interpreters/regex.spec.ts (1)
157-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a params assertion to the regex-dialect cases loop.
The
patternvariable is destructured from each case tuple but never used. Addingexpect(params).toEqual([pattern])would verify thatcreateFilterRegexproduces the expected regex source, strengthening coverage beyond SQL shape alone.♻️ Proposed addition
const [sql, params] = adapter.getQueryAndParameters(); expect(sql, operator).toEqual(negated ? '("name" ~* $1 or "name" is null)' : '"name" ~* $1'); + expect(params, operator).toEqual([pattern]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sql/test/unit/interpreters/regex.spec.ts` around lines 157 - 175, Add a parameter assertion inside the regex cases loop after retrieving adapter output, verifying that params equals the destructured pattern for each operator. Keep the existing SQL assertion unchanged and use the existing pattern variable to validate createFilterRegex output.packages/docs/packages/sql.md (1)
97-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider cross-referencing the two
nintable entries.The existing null-semantics table (L93) shows
nin(field, [a, null])→(field NOT IN (...) AND field IS NOT NULL), while the new complement table (L102) showsnin(field, [a, b])→(field NOT IN (...) OR field IS NULL). Both are correct but represent different scenarios. A brief note distinguishing the null-element case from the all-non-null case would prevent reader confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/docs/packages/sql.md` around lines 97 - 104, Clarify the negated-operator documentation by cross-referencing the existing null-semantics table entry for nin(field, [a, null]) with the complement table’s all-non-null nin(field, [a, b]) example. Add a brief note distinguishing filters containing a NULL element from filters whose values are all non-NULL, preserving both SQL behaviors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/typeorm/test/unit/complement.spec.ts`:
- Around line 36-66: Add an afterAll cleanup hook in the complement test suite
that calls dataSource.destroy() after the beforeAll initialization, ensuring the
initialized DataSource connection is released once all tests complete.
---
Nitpick comments:
In `@packages/docs/packages/sql.md`:
- Around line 97-104: Clarify the negated-operator documentation by
cross-referencing the existing null-semantics table entry for nin(field, [a,
null]) with the complement table’s all-non-null nin(field, [a, b]) example. Add
a brief note distinguishing filters containing a NULL element from filters whose
values are all non-NULL, preserving both SQL behaviors.
In `@packages/sql/test/unit/interpreters/regex.spec.ts`:
- Around line 157-175: Add a parameter assertion inside the regex cases loop
after retrieving adapter output, verifying that params equals the destructured
pattern for each operator. Keep the existing SQL assertion unchanged and use the
existing pattern variable to validate createFilterRegex output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7296b72d-2fb0-4c08-9737-20ec604648f8
📒 Files selected for processing (9)
packages/docs/guide/filters.mdpackages/docs/packages/memory.mdpackages/docs/packages/sql.mdpackages/sql/src/visitor/filters.tspackages/sql/test/unit/interpreters/in.spec.tspackages/sql/test/unit/interpreters/primitves.spec.tspackages/sql/test/unit/interpreters/regex.spec.tspackages/typeorm/test/unit/complement.spec.tspackages/typeorm/test/unit/filters.spec.ts
💤 Files with no reviewable changes (1)
- packages/docs/packages/memory.md
This was referenced Jul 14, 2026
Closed
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Negated filter operators evaluated through
@rapiq/sql/@rapiq/typeormsilently excluded records whose column isNULL:ne('name', 'Peter')rendered a barename <> ?, which dropsNULLrows under SQL three-valued logic. Every negated operator is now the exact logical complement of its positive twin — matching the semantics@rapiq/memoryalready pins with its complement-law spec.A shared
whereComplement()helper in the filters visitor wraps every negated rendering:ne(field, a)"field" <> $1("field" <> $1 or "field" is null)nin(field, [a, b])"field" not in($1, $2)("field" not in($1, $2) or "field" is null)notContainsfamily (regexp dialects)"field" ~* $1("field" ~* $1 or "field" is null)notContainsfamily (LIKE fallback)[field] not like ? escape '\'([field] not like ? escape '\' or [field] is null)The already-complement-correct renderings are unchanged:
ne(null)→is not null,ninwith a null element →(not in(...) and is not null), only-null / empty-listnin, andexists(false).The TypeORM adapter contains no rendering logic of its own, so it inherits the fix 1:1 through the shared visitor.
Tests
ne,nin, negated anchored operators on both the regexp path and the LIKE fallback); the existing null-semantics spec stayed green throughout.NULLvalue in the filtered column and assertne,ninandnotContainsreturn it while the positive twins do not.packages/typeorm/test/unit/complement.spec.ts): the complement-law matrix (9 positive/negative pairs, including null-element / empty-listin/ninshapes) evaluated via@rapiq/memoryand via the TypeORM adapter on a live better-sqlite3 database yields identical record sets, and each pair partitions the table.@rapiq/memoryresolves through the workspace symlink — no dependency declaration needed for the test-only import.Docs
@rapiq/sqlpage: null-semantics section documents the null-inclusive negations; string-matching section notes the LIKE-fallback form.@rapiq/memorypage: removed the "alignment planned" divergence row.Closes #752
Summary by CodeRabbit
Bug Fixes
ne,nin, and negated string matching) so they are null-inclusive and correctly match records with missing orNULLfield values.Documentation
NULL/absent fields.Tests