Skip to content

[2.10] MOD-13010: Fix cmp_strings() - correctly handle binary data with embedded NULLS#7771

Merged
nafraf merged 1 commit into2.10from
nafraf_backport-7765-to-2.10
Dec 15, 2025
Merged

[2.10] MOD-13010: Fix cmp_strings() - correctly handle binary data with embedded NULLS#7771
nafraf merged 1 commit into2.10from
nafraf_backport-7765-to-2.10

Conversation

@nafraf
Copy link
Collaborator

@nafraf nafraf commented Dec 14, 2025

Manual backport #7765 to 2.10

Manual backport was needed due to different location of value.c file:

  • in master: src/value/value.c
  • in 2.10: src/value.c

Note

Switch string comparison to memcmp to handle embedded NULLs and add a test verifying aggregate/groupby coherence with binary data.

  • Core:
    • Update cmp_strings in src/value.c to use memcmp (instead of strncmp) for length-aware, binary-safe comparisons (handles embedded \0).
  • Tests:
    • Add test_mod_13010 in tests/pytests/test_issues.py to validate consistent results between FT.AGGREGATE queries (with/without GROUPBY/TOLIST) when field values contain embedded NULL bytes.

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

@nafraf nafraf requested review from kei-nan and oshadmi December 14, 2025 17:03
@nafraf nafraf marked this pull request as ready for review December 14, 2025 17:03
@codecov
Copy link

codecov bot commented Dec 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.41%. Comparing base (816fbb3) to head (3fc57cb).
⚠️ Report is 3 commits behind head on 2.10.

Additional details and impacted files
@@            Coverage Diff             @@
##             2.10    #7771      +/-   ##
==========================================
- Coverage   89.41%   89.41%   -0.01%     
==========================================
  Files         210      210              
  Lines       36138    36138              
==========================================
- Hits        32314    32311       -3     
- Misses       3824     3827       +3     
Flag Coverage Δ
flow 84.06% <100.00%> (-0.14%) ⬇️
unit 42.08% <100.00%> (ø)

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.

@nafraf nafraf added this pull request to the merge queue Dec 15, 2025
Merged via the queue into 2.10 with commit 82db984 Dec 15, 2025
20 of 22 checks passed
@nafraf nafraf deleted the nafraf_backport-7765-to-2.10 branch December 15, 2025 13:05
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.

3 participants