MOD-11358 integrate ijson to the json module by AvivDavid23 · Pull Request #1394 · RedisJSON/RedisJSON

AvivDavid23 · 2025-09-29T06:52:32Z

Support returning either Owned or Borrowed values from iterators, leading to whole changes in code flow.
Changing c_api, introducing a new api to allow to always return a reference to the underlying data(since we can't always return a reference to the SelectValue from the iterators themselves
Add benchmark file

Note

Refactors JSON path/value handling to support borrowed/owned values, adds typed numeric-array operations, and upgrades the C API to V6 with new allocation and access patterns, alongside dependency bumps and tests/benchmarks.

Core (json_path/select_value):
- Introduce ValueRef to return either borrowed or owned values; update SelectValue trait methods (values, items, get_key, get_index) and equality via is_equal.
- Refactor PathCalculator/results to use ValueRef; add helper macros to traverse ValueRef; adjust filter, scan, and iterator logic.
- Update calc_once/public APIs and CLI output to handle ValueRef.
Redis Module (redis_json):
- Implement typed numeric-array support in ivalue_manager (e.g., i8/u8/i16/u16/f16/bf16/i32/u32/f32/i64/u64/f64) for NUMINCRBY, ARRINSERT, ARRTRIM, etc.
- Overhaul C API to V6: add allocJson/freeJson, change getAt and nextKeyValue to write via pointers and return status; wrap values with ValueWrapper to manage ownership; export new symbols.
- Adapt commands and helpers (KeyValue, type checks, RESP3 serialization) to ValueRef; use shared is_equal.
Dependencies:
- Bump ijson revision; add half (and transitive crunchy, paste).
Tests/Benchmarks:
- Add tests for C API wrappers and numeric array operations; tweak expected memory values and defrag thresholds.
- Add homogeneous_integer_arrays.json dataset and json_set_homogeneous_integer_arrays.yml benchmark.
Misc:
- Minor tooling/script fixes (logs path).

^{Written by Cursor Bugbot for commit 7576f5e. This will update automatically on new commits. Configure here.}

codecov · 2025-09-29T07:01:20Z

Codecov Report

❌ Patch coverage is 66.74528% with 141 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.09%. Comparing base (93ff948) to head (7576f5e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
redis_json/src/c_api.rs	1.16%	85 Missing ⚠️
redis_json/src/ivalue_manager.rs	80.83%	23 Missing ⚠️
json_path/src/json_path.rs	80.43%	18 Missing ⚠️
json_path/src/select_value.rs	82.05%	7 Missing ⚠️
json_path/src/json_node.rs	77.77%	6 Missing ⚠️
redis_json/src/key_value.rs	93.93%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1394      +/-   ##
==========================================
- Coverage   79.28%   78.09%   -1.19%     
==========================================
  Files          15       16       +1     
  Lines        3707     3908     +201     
==========================================
+ Hits         2939     3052     +113     
- Misses        768      856      +88

☔ 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.

eranhd · 2025-09-29T07:06:24Z

@AvivDavid23 what happens to the coverage?

redis_json/src/commands.rs

redis_json/src/c_api.rs

json_path/src/json_node.rs

redis_json/src/ivalue_manager.rs

json_path/src/json_path.rs

json_path/src/json_node.rs

redis_json/src/include/rejson_api.h

gabsow · 2025-10-09T14:54:12Z

Whats our status?
Can we merge this PR ?

This reverts commit 5af50a4.

This reverts commit 852a223.

This reverts commit 4252d45.

This reverts commit d3e8e5518fea0aa0fcde5e781f7f68c9bffbc1ea.

This reverts commit 3056dfc5b907eca917267dee636facfcfab82102.

This reverts commit cd40fd7aa2db86ca51e82a0604f215f8fbcc5ade.

json_path/src/select_value.rs

* initial, but tests fails * change all to clone for testing * Revert "change all to clone for testing" This reverts commit 5af50a4. * Revert "Revert "change all to clone for testing"" This reverts commit 852a223. * Revert "Revert "Revert "change all to clone for testing""" This reverts commit 4252d45. * complies, tests are failing * better macro * debug * fix values set for arrays * Revert "debug" This reverts commit d3e8e5518fea0aa0fcde5e781f7f68c9bffbc1ea. * remove print * Revert "Revert "debug"" This reverts commit 3056dfc5b907eca917267dee636facfcfab82102. * fix tests * Revert "Revert "Revert "debug""" This reverts commit cd40fd7aa2db86ca51e82a0604f215f8fbcc5ade. * c_api impl * cargo fmt * register c_api ver 6 * add another test * test * fix * more coverage * more coverage * more coverage * test also integer + float numerics * remove get_at c api * bump ijson * update c_api * fix * CR * cargo fmt * c_api * fmt * free data * c_api * header fix * cr * add benchmark * c_api * refine test * refine test * add json_api_get_value_from_ptr * fix c_api * fmt * fix bench * move is_equal of select values * fix (cherry picked from commit 9a5f051)

GuyAv46 · 2025-10-15T13:09:25Z

redis_json/src/c_api.rs

                ));

-                for v in 1..6 {
+                for v in 1..7 {


We should not register API versions we can't supply

Suggested change

for v in 1..7 {

for v in 6..=6 {

i don't understand. we're not supplying v7 nor are we claiming to. ranges are defined to be half open. 1..7 has 6 elements; {1, 2, 3, 4, 5, 6}

The =6 is a nitpick, I think it's clearer than 7
But I'm talking about the minimal API - the module cannot supply v1-v5 anymore.
Currently, a module can register to any of 1-5 and crash the first time it uses it. If you only expose v6 now, outdated modules will simply not link to JSON and behave as if it's missing.
The range in this for loop should only include 1 element - {6}

AvivDavid23 requested a review from ephraimfeldblum September 29, 2025 14:37

AvivDavid23 force-pushed the MOD-11358-integrate-ijson-to-the-json-module-avivd branch 2 times, most recently from d8e4be1 to 06d0a2d Compare October 5, 2025 06:58

ephraimfeldblum suggested changes Oct 5, 2025

View reviewed changes

AvivDavid23 requested a review from ephraimfeldblum October 8, 2025 16:42

ephraimfeldblum reviewed Oct 9, 2025

View reviewed changes

AvivDavid23 force-pushed the MOD-11358-integrate-ijson-to-the-json-module-avivd branch from d2037f3 to 6b30a0a Compare October 9, 2025 07:24

oshadmi reviewed Oct 9, 2025

View reviewed changes

redis_json/src/include/rejson_api.h Outdated Show resolved Hide resolved

AvivDavid23 added 19 commits October 13, 2025 12:16

initial, but tests fails

ad75d48

change all to clone for testing

b71d056

Revert "change all to clone for testing"

a97daad

This reverts commit 5af50a4.

Revert "Revert "change all to clone for testing""

66e77ce

This reverts commit 852a223.

Revert "Revert "Revert "change all to clone for testing"""

d4544fa

This reverts commit 4252d45.

complies, tests are failing

26387ee

better macro

44168f8

debug

a59fea3

fix values set for arrays

27a9122

Revert "debug"

443bcbd

This reverts commit d3e8e5518fea0aa0fcde5e781f7f68c9bffbc1ea.

remove print

d1d8f82

Revert "Revert "debug""

ea4bab0

This reverts commit 3056dfc5b907eca917267dee636facfcfab82102.

fix tests

09149ce

Revert "Revert "Revert "debug"""

4a2b7c1

This reverts commit cd40fd7aa2db86ca51e82a0604f215f8fbcc5ade.

c_api impl

abaf2db

cargo fmt

e256438

register c_api ver 6

25ff827

add another test

e2a0c50

test

1dfe692

AvivDavid23 added 16 commits October 13, 2025 12:16

fix

abff074

CR

b8a0399

cargo fmt

bdaeb53

c_api

35fc0f6

fmt

eb5a054

free data

d325664

c_api

f1c08d2

header fix

0d8452c

cr

e940779

add benchmark

1717890

c_api

9082afc

refine test

2cab6b0

refine test

10d6f76

add json_api_get_value_from_ptr

e03e769

fix c_api

ffa3502

fmt

274a91e

AvivDavid23 force-pushed the MOD-11358-integrate-ijson-to-the-json-module-avivd branch from ae81ba1 to 274a91e Compare October 13, 2025 09:17

AvivDavid23 requested a review from ephraimfeldblum October 13, 2025 09:17

fix bench

be31eca

ephraimfeldblum reviewed Oct 15, 2025

View reviewed changes

json_path/src/select_value.rs Show resolved Hide resolved

move is_equal of select values

1e4cb63

This comment was marked as outdated.

Sign in to view

fix

7576f5e

ephraimfeldblum approved these changes Oct 15, 2025

View reviewed changes

AvivDavid23 merged commit 9a5f051 into master Oct 15, 2025
19 of 21 checks passed

AvivDavid23 deleted the MOD-11358-integrate-ijson-to-the-json-module-avivd branch October 15, 2025 10:01

GuyAv46 mentioned this pull request Oct 15, 2025

Update JSON LLAPI - [MOD-11606] RediSearch/RediSearch#6920

Merged

2 tasks

GuyAv46 reviewed Oct 15, 2025

View reviewed changes

Conversation

AvivDavid23 commented Sep 29, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

eranhd commented Sep 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gabsow commented Oct 9, 2025

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

GuyAv46 Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

ephraimfeldblum Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

GuyAv46 Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

AvivDavid23 commented Sep 29, 2025 •

edited by cursor bot

Loading

codecov bot commented Sep 29, 2025 •

edited

Loading

GuyAv46 Oct 15, 2025 •

edited

Loading