Skip to content

[2.x] fix: notification dedup query never matches non-null data on MySQL - #4645

Merged
imorland merged 3 commits into
2.xfrom
im/notification-dedup-mysql-json
May 9, 2026
Merged

[2.x] fix: notification dedup query never matches non-null data on MySQL#4645
imorland merged 3 commits into
2.xfrom
im/notification-dedup-mysql-json

Conversation

@imorland

@imorland imorland commented May 8, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #4643. `Notification::scopeMatchingBlueprint()` on MySQL never matches existing rows when the blueprint's `data` is non-null, silently disabling dedup for every notification type whose blueprint returns non-null `getData()` — `postMentioned`, `newPost`, `postMoved`, plus extension types like `byobuPrivateDiscussionReplied`. Every edit of a post that triggered such a notification creates a new row and re-sends the email.

Root cause

Before:

```php
return $query->where($attributes)
->whenPgSql(function ($query) use ($data) {
return $query->whereRaw('data::text = ?', [$data]);
}, function ($query) use ($data) {
return $query->where('data', $data);
});
```

The PostgreSQL branch correctly casts the column to text. The else branch (MySQL / MariaDB / SQLite) compares the JSON column directly to a string. `getBlueprintAttributes()` produces `$data` via `json_encode($data)` — compact form like `{"replyNumber":58}`. MySQL stores its JSON column canonicalised — with a space after the colon: `{"replyNumber": 58}`. Direct string comparison never matches.

Fix

Branch by driver explicitly:

  • PostgreSQL — cast column to text (`data::text = ?`). Unchanged.
  • MySQL — cast parameter to JSON (`data = CAST(? AS JSON)`) so the comparison round-trips through MySQL's canonical form.
  • MariaDB — its JSON type is just LONGTEXT and `CAST(... AS JSON)` isn't supported syntax. The bytes round-trip and plain string equality works.
  • SQLite — JSON stored as plain TEXT, byte-for-byte round-trip, plain string equality works.

The null-data path is pulled out front since `= NULL` doesn't match anything on any engine.

Tests

Three integration tests in `framework/core/tests/integration/notification/MatchingBlueprintTest.php`:

  1. `matchingBlueprint()` finds existing rows when blueprint data is null (sanity check).
  2. `matchingBlueprint()` finds existing rows when blueprint data is non-null (the regression).
  3. `NotificationSyncer::sync()` called repeatedly with the same blueprint+recipient produces exactly one row, not duplicates (the user-visible symptom).

The first commit added these tests without the fix, so the CI matrix demonstrated which engines were affected (MySQL fail; PostgreSQL/MariaDB/SQLite pass — confirming the diagnosis). The second commit applied the fix; full matrix now green.

Scale

From the issue reporter's production instance:

```
type dup_groups total_dup_rows
postMentioned 665 1,477
newPost 2 4
```

`postMentioned` is the dominant offender precisely because it has non-null data and the dedup query was broken for it.

Relationship to #4622

#4622 describes a queue-timing race in `NotificationSyncer::sync()` for null-data blueprints (where the dedup query does work, so duplicates only arise from a narrow window). For non-null-data blueprints, this bug means duplicates were guaranteed on every edit regardless of timing — much higher impact, much simpler fix. Both are needed; this is the one users hit most often.

imorland added 2 commits May 8, 2026 17:42
Adds three integration tests for the model scope and the user-visible
syncer behaviour:

1. matchingBlueprint() finds existing rows when blueprint data is null
   (sanity check — the path that does work today)
2. matchingBlueprint() finds existing rows when blueprint data is non-null
   — currently fails on MySQL because the scope compares the JSON column
   directly to the json_encode'd string, but MySQL canonicalises stored
   JSON ({"replyNumber": 58}) and the encoded input is compact
   ({"replyNumber":58}), so string equality never matches
3. NotificationSyncer::sync() called repeatedly with the same blueprint
   does not create duplicate rows — the user-visible symptom on every
   edit of a post that triggered postMentioned, newPost, postMoved, etc.

See #4643. Pushing the tests
without the fix so the CI matrix demonstrates which DB engines hit the
bug.
The scope used `->where('data', $data)` which on MySQL compares a JSON
column directly to a string. MySQL stores JSON canonicalised — `{"a": 1}`
even when the input was `{"a":1}`, which `json_encode()` produces — so
string equality never matches and every NotificationSyncer::sync() call
on a blueprint with non-null data treated the recipient as new and
re-emailed them.

Branch by driver:
- PostgreSQL: cast column to text (`data::text = ?`) — unchanged
- MySQL/MariaDB: cast parameter to JSON (`data = CAST(? AS JSON)`) so
  the comparison round-trips through MySQL's canonical form
- SQLite: stores JSON as opaque text, plain `=` works as before

Pull the null-data path out front since `= NULL` doesn't match anything
on any engine and there's no driver-specific handling needed.

Fixes #4643.
@imorland imorland added this to the 2.0.0-rc.2 milestone May 9, 2026
MariaDB rejects CAST(... AS JSON) syntax — its JSON column is just an
alias for LONGTEXT and there's no SQL-standard JSON cast. The previous
fix incorrectly applied that cast to MariaDB too, breaking the matrix
on MariaDB and MariaDB-prefix.

Branch by driver explicitly:
- PostgreSQL: data::text = ? (cast column to text)
- MySQL: data = CAST(? AS JSON) (cast parameter to JSON)
- MariaDB / SQLite: plain string equality (no canonicalisation)
@imorland
imorland marked this pull request as ready for review May 9, 2026 05:18
@imorland
imorland requested a review from a team as a code owner May 9, 2026 05:18
@imorland
imorland merged commit 690c2fa into 2.x May 9, 2026
25 checks passed
@imorland
imorland deleted the im/notification-dedup-mysql-json branch May 9, 2026 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notification dedup query (matchingBlueprint) never matches when data is non-null on MySQL — duplicates on every post edit

1 participant