Review jsonpath_rs (2)

[oshadmi]

Replace #10

Few comments up til now:

• Do we need to support/test filter with full_scan or all? (currently it seems to be allowed by the grammar)
• We should probably add more comments in code
• Did not get the flat_arrays_on_filter - you mean we should not iterate all array entries, but only the relevant ones (e.g., if using a union, no need to iterate all?)
  And not sure I understand, for example, in calc_internal in Rule::filter in the else clause we pass true and not the param we got flat_arrays_on_filter
• Added tests which panicked (filter_inner, filter_nested) so they are commented out to be reviewed later

[MeirShpilraien]

Do we need to support/test filter with full_scan or all? (currently it seems to be allowed by the grammar)

Yes, but lets leave it to a separate PR

We should probably add more comments in code

Agree, will add it on this PR.

Did not get the flat_arrays_on_filter - you mean we should not iterate all array entries, but only the relevant ones (e.g., if using a union, no need to iterate all?)
And not sure I understand, for example, in calc_internal in Rule::filter in the else clause we pass true and not the param we got flat_arrays_on_filter

The json path rules was not clear to me, But I believe I understand now, if the filter is applied on simple value (string, number, boolean, ...) then it is applied on that value. But if it is applied on array or object, it is applies on each element. I will change the code, we might not need the flat_arrays_on_filter at all.

Added tests which panicked (filter_inner, filter_nested) so they are commented out to be reviewed later

Saw that, will fix them.

[oshadmi]

👍🏼 Few comments

[oshadmi]

Is there any impact on performance from this change? (always flattening also without filter)

[MeirShpilraien]

Not that I can think of...

[MeirShpilraien]

@oshadmi fixed.