Skip to content

Commit 552b794

Browse files
gkorlandoshadmi
gkorland
and
oshadmi
authored
Support multi value in arrindex (RedisJSON#473)
* initial commit for multi index * fix case when nothing is found in the array * format code * add tests and fix test * Multi path: arrindex handle multi * Multi path: arrindex handle multi - fix tests and update doc * Multi path: arrindex - `stop` index can be negative * Multi path: arrindex - per Guy's review * Multi path: arrindex - fix legacy test with none-scalar value * Multi path: arrindex - fixes per Gavrie's review * Multi path: arrindex - fixes per Gavrie's review (2) Co-authored-by: oshadmi <omer.shadmi@redislabs.com>
1 parent 473f09c commit 552b794

File tree

7 files changed

+240
-67
lines changed

7 files changed

+240
-67
lines changed

‎docs/commands.md

+4-1
Original file line numberDiff line numberDiff line change
@@ -253,12 +253,14 @@ JSON.ARRINDEX <key> <path> <json-scalar> [start [stop]]
253253
Search for the first occurrence of a scalar JSON value in an array.
254254

255255
The optional inclusive `start` (default 0) and exclusive `stop` (default 0, meaning that the last element is included) specify a slice of the array to search.
256+
Negative values are interpreted as starting from the end.
257+
256258

257259
Note: out of range errors are treated by rounding the index to the array's start and end. An inverse index range (e.g. from 1 to 0) will return unfound.
258260

259261
#### Return value
260262

261-
[Integer][2], specifically the position of the scalar value in the array, or -1 if unfound.
263+
[Array][4], specifically, for each JSON value matching the path, the first position of the scalar value in the array, -1 if unfound in the array, or [null][6] element if the matching JSON value is not an array.
262264

263265
### JSON.ARRINSERT
264266

@@ -474,3 +476,4 @@ Return the JSON in `key` in [Redis Serialization Protocol (RESP)][5].
474476
[3]: http://redis.io/topics/protocol#resp-bulk-strings
475477
[4]: http://redis.io/topics/protocol#resp-arrays
476478
[5]: http://redis.io/topics/protocol
479+
[6]: https://redis.io/topics/protocol#null-elements-in-arrays

‎src/commands.rs

+87-45
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::formatter::RedisJsonFormatter;
22
use crate::manager::{AddUpdateInfo, Manager, ReadHolder, SetUpdateInfo, UpdateInfo, WriteHolder};
3-
use crate::redisjson::{Format, Path};
3+
use crate::redisjson::{normalize_arr_indices, Format, Path};
44
use jsonpath_lib::select::select_value::{SelectValue, SelectValueType};
55
use redis_module::{Context, RedisValue};
66
use redis_module::{NextArg, RedisError, RedisResult, RedisString, REDIS_OK};
@@ -315,7 +315,7 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
315315
}
316316
}
317317

318-
pub fn is_eqaul<T1: SelectValue, T2: SelectValue>(&self, a: &T1, b: &T2) -> bool {
318+
pub fn is_equal<T1: SelectValue, T2: SelectValue>(&self, a: &T1, b: &T2) -> bool {
319319
match (a.get_type(), b.get_type()) {
320320
(SelectValueType::Null, SelectValueType::Null) => true,
321321
(SelectValueType::Bool, SelectValueType::Bool) => a.get_bool() == b.get_bool(),
@@ -327,7 +327,7 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
327327
false
328328
} else {
329329
for (i, e) in a.values().unwrap().into_iter().enumerate() {
330-
if !self.is_eqaul(e, b.get_index(i).unwrap()) {
330+
if !self.is_equal(e, b.get_index(i).unwrap()) {
331331
return false;
332332
}
333333
}
@@ -343,7 +343,7 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
343343
let temp2 = b.get_key(k);
344344
match (temp1, temp2) {
345345
(Some(a1), Some(b1)) => {
346-
if !self.is_eqaul(a1, b1) {
346+
if !self.is_equal(a1, b1) {
347347
return false;
348348
}
349349
}
@@ -360,51 +360,62 @@ impl<'a, V: SelectValue> KeyValue<'a, V> {
360360
pub fn arr_index(
361361
&self,
362362
path: &str,
363-
scalar_json: &str,
363+
scalar_value: Value,
364364
start: i64,
365365
end: i64,
366-
) -> Result<i64, Error> {
367-
let res = self.get_first(path)?;
368-
if res.get_type() == SelectValueType::Array {
369-
// end=-1/0 means INFINITY to support backward with RedisJSON
370-
if res.len().unwrap() == 0 || end < -1 {
371-
return Ok(-1);
372-
}
373-
let v: Value = serde_json::from_str(scalar_json)?;
366+
) -> Result<RedisValue, Error> {
367+
let res = self
368+
.get_values(path)?
369+
.iter()
370+
.map(|value| {
371+
self.arr_first_index_single(value, &scalar_value, start, end)
372+
.into()
373+
})
374+
.collect::<Vec<RedisValue>>();
375+
Ok(res.into())
376+
}
374377

375-
let len = res.len().unwrap() as i64;
378+
pub fn arr_index_legacy(
379+
&self,
380+
path: &str,
381+
scalar_value: Value,
382+
start: i64,
383+
end: i64,
384+
) -> Result<RedisValue, Error> {
385+
let arr = self.get_first(path)?;
386+
Ok(
387+
match self.arr_first_index_single(arr, &scalar_value, start, end) {
388+
FoundIndex::NotArray => RedisValue::Integer(-1),
389+
i => i.into(),
390+
},
391+
)
392+
}
376393

377-
// Normalize start
378-
let start = if start < 0 {
379-
0.max(len + start)
380-
} else {
381-
// start >= 0
382-
start.min(len - 1)
383-
};
394+
/// Returns first array index of `v` in `arr`, or NotFound if not found in `arr`, or NotArray if `arr` is not an array
395+
fn arr_first_index_single(&self, arr: &V, v: &Value, start: i64, end: i64) -> FoundIndex {
396+
if !arr.is_array() {
397+
return FoundIndex::NotArray;
398+
}
384399

385-
// Normalize end
386-
let end = match end {
387-
0 => len,
388-
e if e < 0 => len + end,
389-
_ => end.min(len),
390-
};
400+
let len = arr.len().unwrap() as i64;
401+
if len == 0 {
402+
return FoundIndex::NotFound;
403+
}
404+
// end=0 means INFINITY to support backward with RedisJSON
405+
let (start, end) = normalize_arr_indices(start, end, len);
391406

392-
if end < start {
393-
// don't search at all
394-
return Ok(-1);
395-
}
396-
let mut i = -1;
397-
for index in start..end {
398-
if self.is_eqaul(res.get_index(index as usize).unwrap(), &v) {
399-
i = index;
400-
break;
401-
}
402-
}
407+
if end < start {
408+
// don't search at all
409+
return FoundIndex::NotFound;
410+
}
403411

404-
Ok(i)
405-
} else {
406-
Ok(-1)
412+
for index in start..end {
413+
if self.is_equal(arr.get_index(index as usize).unwrap(), v) {
414+
return FoundIndex::Index(index);
415+
}
407416
}
417+
418+
FoundIndex::NotFound
408419
}
409420

410421
pub fn obj_keys(&self, path: &str) -> Result<Box<dyn Iterator<Item = &'_ str> + '_>, Error> {
@@ -868,6 +879,22 @@ pub fn command_json_arr_append<M: Manager>(
868879
}
869880
}
870881

882+
enum FoundIndex {
883+
Index(i64),
884+
NotFound,
885+
NotArray,
886+
}
887+
888+
impl From<FoundIndex> for RedisValue {
889+
fn from(e: FoundIndex) -> Self {
890+
match e {
891+
FoundIndex::NotFound => RedisValue::Integer(-1),
892+
FoundIndex::NotArray => RedisValue::Null,
893+
FoundIndex::Index(i) => RedisValue::Integer(i),
894+
}
895+
}
896+
}
897+
871898
pub fn command_json_arr_index<M: Manager>(
872899
manager: M,
873900
ctx: &Context,
@@ -885,11 +912,26 @@ pub fn command_json_arr_index<M: Manager>(
885912

886913
let key = manager.open_key_read(ctx, &key)?;
887914

888-
let index = key.get_value()?.map_or(Ok(-1), |doc| {
889-
KeyValue::new(doc).arr_index(path.get_path(), json_scalar, start, end)
890-
})?;
915+
let is_legacy = path.is_legacy();
916+
let scalar_value: Value = serde_json::from_str(json_scalar)?;
917+
if !is_legacy && (scalar_value.is_array() || scalar_value.is_object()) {
918+
return Err(RedisError::String(format!(
919+
"ERR expected scalar but found {}",
920+
json_scalar
921+
)));
922+
}
923+
924+
let res = key
925+
.get_value()?
926+
.map_or(Ok(RedisValue::Integer(-1)), |doc| {
927+
if path.is_legacy() {
928+
KeyValue::new(doc).arr_index_legacy(path.get_path(), scalar_value, start, end)
929+
} else {
930+
KeyValue::new(doc).arr_index(path.get_path(), scalar_value, start, end)
931+
}
932+
})?;
891933

892-
Ok(index.into())
934+
Ok(res)
893935
}
894936

895937
pub fn command_json_arr_insert<M: Manager>(

‎src/manager.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use redis_module::{Context, NotifyEvent, RedisString};
99

1010
use std::marker::PhantomData;
1111

12-
use crate::redisjson::RedisJSON;
12+
use crate::redisjson::{normalize_arr_start_index, RedisJSON};
1313
use crate::Format;
1414
use crate::REDIS_JSON_TYPE;
1515

@@ -417,11 +417,7 @@ impl<'a> WriteHolder<Value, Value> for KeyHolderWrite<'a> {
417417
}
418418
// Verify legel index in bounds
419419
let len = array.len() as i64;
420-
let index = if index < 0 {
421-
0.max(len + index)
422-
} else {
423-
index.min(len - 1)
424-
} as usize;
420+
let index = normalize_arr_start_index(index, len) as usize;
425421

426422
let mut new_value = v.take();
427423
let curr = new_value.as_array_mut().unwrap();

‎src/redisjson.rs

+25-14
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,33 @@ use crate::c_api::JSONType;
1919
use crate::error::Error;
2020
use crate::nodevisitor::{StaticPathElement, StaticPathParser, VisitStatus};
2121

22+
//use crate::normalize_arr_start_index;
2223
use std::fmt;
2324
use std::fmt::Display;
2425

26+
/// Returns normalized start index
27+
pub fn normalize_arr_start_index(start: i64, len: i64) -> i64 {
28+
if start < 0 {
29+
0.max(len + start)
30+
} else {
31+
// start >= 0
32+
start.min(len - 1)
33+
}
34+
}
35+
36+
/// Return normalized `(start, end)` indices as a tuple
37+
pub fn normalize_arr_indices(start: i64, end: i64, len: i64) -> (i64, i64) {
38+
// Normalize start
39+
let start = normalize_arr_start_index(start, len);
40+
// Normalize end
41+
let end = match end {
42+
0 => len,
43+
e if e < 0 => len + end,
44+
_ => end.min(len),
45+
};
46+
(start, end)
47+
}
48+
2549
#[derive(Debug, PartialEq)]
2650
pub enum SetOptions {
2751
NotExists,
@@ -308,20 +332,7 @@ impl RedisJSON {
308332

309333
let len = arr.len() as i64;
310334

311-
// Normalize start
312-
let start = if start < 0 {
313-
0.max(len + start)
314-
} else {
315-
// start >= 0
316-
start.min(len - 1)
317-
};
318-
319-
// Normalize end
320-
let end = match end {
321-
0 => len,
322-
e if e < 0 => len + end,
323-
_ => end.min(len),
324-
};
335+
let (start, end) = normalize_arr_indices(start, end, len);
325336

326337
if end < start {
327338
// don't search at all

‎tests/pytest/test.py

+9
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,7 @@ def testArrIndexCommand(env):
592592
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 6), 6)
593593
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 4, -0), 6)
594594
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 5, -1), -1)
595+
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 0, 5, 0), 6)
595596
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 2, -2, 6), -1)
596597
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '"foo"'), -1)
597598

@@ -600,6 +601,14 @@ def testArrIndexCommand(env):
600601
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 2, 3), 5)
601602
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', '[4]'), 4)
602603

604+
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '.arr', 1), 1)
605+
606+
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '$.arr', 1), [1])
607+
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '$.arr', 2, 1, 4), [2])
608+
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '$.arr', 6), [-1])
609+
r.assertEqual(r.execute_command('JSON.ARRINDEX', 'test', '$.arr', 3, 0, 2), [-1])
610+
611+
603612
def testArrInsertCommand(env):
604613
"""Test JSON.ARRINSERT command"""
605614
r = env

‎tests/pytest/test_keyspace_notifications.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def test_keyspace_arr(env):
8080
assert_msg(env, pubsub.get_message(), 'pmessage', 'test_key_arr')
8181

8282
# Negative tests should not get an event
83-
env.assertEqual(0, r.execute_command('JSON.ARRINDEX', 'test_key_arr', '$.foo', '"gogo1"'))
83+
env.assertEqual([0], r.execute_command('JSON.ARRINDEX', 'test_key_arr', '$.foo', '"gogo1"'))
8484
env.assertEqual(None, pubsub.get_message())
8585

8686
env.assertEqual(2, r.execute_command('JSON.ARRLEN', 'test_key_arr', '$.foo'))

0 commit comments

Comments
 (0)