MOD-14328: [8.6] Notification handler for rename is loading the wrong key#8589
MOD-14328: [8.6] Notification handler for rename is loading the wrong key#8589
rename is loading the wrong key#8589Conversation
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| if (runFilters) { | ||
| // We load the data from the `keyToReadData` key, which is the key the old | ||
| // key was changed to, since the old key is already deleted. | ||
| key_p = RedisModule_StringPtrLen(keyToReadData, NULL); |
There was a problem hiding this comment.
Missing GitHub release note for this PR
Low Severity
Per project review rules, a GitHub release note is needed for this PR. Suggested release note: "Fixed a bug where the RENAME notification handler for indexes with FILTER expressions would attempt to read field data from the already-deleted old key instead of the new key, causing filters to fail during rename operations between indexes."
Triggered by project rule: Please assist in writing a GitHub release note for this PR, which is concise and focused on the user impact.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.6 #8589 +/- ##
==========================================
+ Coverage 83.83% 83.85% +0.02%
==========================================
Files 367 367
Lines 55649 55650 +1
Branches 14319 14319
==========================================
+ Hits 46656 46668 +12
+ Misses 8832 8821 -11
Partials 161 161
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
raz-mon
left a comment
raz-mon
left a comment
There was a problem hiding this comment.
Reviewed and approved by myself (can't approve my own PR)
2 participants
Backport of #8377 to 8.6 branch.
Original ticket: MOD-14062
Backport ticket: MOD-14328
Co-authored by Augment Code
Pull Request opened by Augment Code with guidance from the PR author
Note
Medium Risk
Touches keyspace notification rename handling and filter evaluation, which can affect whether documents are (de)indexed during
RENAMEacross prefixes. Risk is mitigated by added coverage for cross-index renames with field-based filters and no-op renames.Overview
Fixes a
RENAMEedge case where indexFILTERexpressions could be evaluated against the wrong key after a rename, causing incorrect indexing (or crashes) when moving documents between index prefixes.Adds regression tests covering renames between indexes with field-value filters (including cases where the target filter should exclude the document) and
RENAMEno-op behavior.Written by Cursor Bugbot for commit b1c5cad. This will update automatically on new commits. Configure here.