Skip to content

[8.4] MOD-14063: Fix FILTER bug with multiple indexes with the same field aliases#8596

Merged
raz-mon merged 2 commits into8.4from
backport-MOD-14063-to-8.4
Mar 10, 2026
Merged

[8.4] MOD-14063: Fix FILTER bug with multiple indexes with the same field aliases#8596
raz-mon merged 2 commits into8.4from
backport-MOD-14063-to-8.4

Conversation

@raz-mon
Copy link
Copy Markdown
Collaborator

@raz-mon raz-mon commented Mar 4, 2026

Summary

Backport of commit 9c70952 from master to 8.4 branch.

This PR fixes a bug where the FILTER expression evaluation was incorrectly sharing field aliases between multiple indexes, causing incorrect indexing behavior.

Related Jira: MOD-14334
Original PR: #8396

Conflicts Resolved

The following conflicts were encountered during the cherry-pick and resolved as follows:

1. src/rlookup.c

  • Conflict: The master branch uses _head/_tail naming convention for RLookup struct members while the 8.4 branch uses head/tail.
  • Resolution: Kept the 8.4 branch naming convention (head/tail) as this is consistent with the existing codebase in this branch. Also kept the memset(lk, 0, sizeof(*lk)) from the incoming change (important for the fix).

2. tests/pytests/test_followhashes.py

  • Conflict: The 8.4 branch contains additional tests that were backported separately (testCountry, testIssue1571, testIssue1571WithRename, testIdxFieldJson, testFilterStartWith, testFilterWithOperator, testFilterWithNot) which were not present in the original commit.
  • Resolution: Preserved all existing 8.4 tests (HEAD version) since these tests are valid and were added as part of previous backports or branch-specific changes.

Pull Request opened by Augment Code with guidance from the PR author


Note

Medium Risk
Touches indexing-time FILTER evaluation and RLookup lifecycle/reset logic; incorrect cleanup could cause missed/extra indexing or memory issues, though changes are localized and covered by new tests.

Overview
Fixes a bug where FILTER expressions could reuse stale RLookup/row state across multiple index evaluations, leading to incorrect indexing when indexes share field aliases.

The filter-evaluation loop now explicitly resets lookup/row state between indexes, and RLookup cleanup/reset is hardened (clear struct to zero, null dyn after free, add an assertion on ndyn). Adds a dedicated test_filter.py suite (and moves prior filter tests out of test_followhashes.py) to cover multi-index/alias scenarios and related filter behavior.

Written by Cursor Bugbot for commit 5fde59b. This will update automatically on new commits. Configure here.

@raz-mon raz-mon requested review from nafraf and oshadmi March 4, 2026 11:10
@github-actions github-actions Bot added the size:L label Mar 4, 2026
@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented Mar 4, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/spec.c
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.50%. Comparing base (b66b4e3) to head (5fde59b).
⚠️ Report is 1 commits behind head on 8.4.

Additional details and impacted files
@@            Coverage Diff             @@
##              8.4    #8596      +/-   ##
==========================================
- Coverage   85.54%   85.50%   -0.05%     
==========================================
  Files         337      337              
  Lines       53353    53356       +3     
  Branches    11023    11023              
==========================================
- Hits        45642    45620      -22     
- Misses       7568     7593      +25     
  Partials      143      143              
Flag Coverage Δ
flow 85.17% <100.00%> (+0.59%) ⬆️
unit 51.06% <60.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… aliases (#8396)

* Fix loaded document in rename flow

* Address review

* Add test

* Fix FILTER bug with multiple indexes with the same field alias
@raz-mon raz-mon force-pushed the backport-MOD-14063-to-8.4 branch from a958943 to a5f2838 Compare March 5, 2026 14:04
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Mar 5, 2026

@raz-mon
Copy link
Copy Markdown
Collaborator Author

raz-mon commented Mar 9, 2026

Approved by me.

@raz-mon raz-mon enabled auto-merge March 9, 2026 14:51
@raz-mon raz-mon requested review from Itzikvaknin and removed request for Itzikvaknin March 10, 2026 08:37
@raz-mon raz-mon added this pull request to the merge queue Mar 10, 2026
Merged via the queue into 8.4 with commit d0203ca Mar 10, 2026
32 of 34 checks passed
@raz-mon raz-mon deleted the backport-MOD-14063-to-8.4 branch March 10, 2026 10:45
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.

2 participants