Skip to content

Fix where_not_null adding a stray binding - #320

Merged
tmgbedu merged 1 commit into
mainfrom
fix/where-not-null-bindings
Oct 26, 2026
Merged

tmgbedu merged 1 commit into
mainfrom
fix/where-not-null-bindings

Conversation

@claude-tmgbedu

Copy link
Copy Markdown

Problem

where_not_null / or_where_not_null added a None binding with no matching ? placeholder. Every later binding shifted by one, so e.g. .where_not_null("x").where("created_at", ">=", d) compared created_at against NULL and returned no rows.

Root cause

BaseGrammar.process_wheres (qmark branch, src/fastapi_startkit/masoniteorm/query/grammars/BaseGrammar.py) excluded NULL clauses from binding but not NOT NULL ones. The builder is correct; the fix is in the grammar, so it applies to all four dialects.

where_column, where_raw without bindings and where_exists/where_has subqueries were checked: they don't add stray bindings themselves, but a where_not_null inside an exists subquery leaked the None to the outer query. The same fix covers that case.

Tests

  • tests/masoniteorm/query/grammars/test_null_where_bindings_grammar.py: for sqlite/mysql/postgres/mssql, checks that the placeholder count equals the binding count and that values stay in order. Covers all four null methods followed by where/where_in/or_where, plus where_column/where_raw and an exists subquery.
  • tests/masoniteorm/sqlite/builder/test_sqlite_null_where_bindings.py: sqlite integration test reproducing the date-filter case.

Note: the builder has no where_date/where_between methods, so the tests use where(col, ">=", date), which hits the same bug.

🤖 Generated with Claude Code

The qmark branch of BaseGrammar.process_wheres skipped the binding for
NULL clauses but not NOT NULL ones, so where_not_null/or_where_not_null
appended a None with no placeholder and shifted every later binding.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@tmgbedu
tmgbedu merged commit 284d097 into main Oct 26, 2026
10 checks passed
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.

2 participants