jq icon indicating copy to clipboard operation
jq copied to clipboard

Update delpaths function to use sort with slicing

Open capak07 opened this issue 1 year ago • 5 comments

Pull Request for #2868

  • Added jv_array_slice function to support slicing of the array

capak07 avatar Feb 12 '24 21:02 capak07

Can someone give an example when this happens? also some test for it would be good

wader avatar Feb 19 '24 23:02 wader

@wader Well, I checked my 2023-08-29 chat logs with @nicowilliams and that issue was about the following jq script (that is supposed to delete duplicate elements in the input array without sorting):

[6,5,1,2,5,3,4,6,7,6,5] |
del(
  . as $out |
  range(length) as $i
  .[$i+1:][] |
  select(. == $dot[$i])
)

Not working as intended because of its use of slices; output:

$ jq -cn '[6,5,1,2,5,3,4,6,7,6,5] | del(. as $dot | range(length) as $i | .[$i+1:][] | select(. == $dot[$i]))'
[6,5,1,2,3,4,7,5]
$ jq -cn '[6,5,1,2,5,3,4,6,7,6,5] | delpaths([path(. as $dot | range(length) as $i | .[$i+1:][] | select(. == $dot[$i])) | debug])'
["DEBUG:",[{"start":1,"end":null},6]]
["DEBUG:",[{"start":1,"end":null},8]]
["DEBUG:",[{"start":2,"end":null},2]]
["DEBUG:",[{"start":2,"end":null},8]]
["DEBUG:",[{"start":5,"end":null},5]]
["DEBUG:",[{"start":8,"end":null},1]]
[6,5,1,2,3,4,7,5]

(The last duplicate 5 did not get deleted)

This is the usual problem that afflicts path expressions with negative indices and slices; the slice paths do not get canonicalised (or they are not sorted in an order that would make the deletions work correctly), so they end up pointing the wrong elements.

A version of that same script that uses a range to loop the array instead of a slice works correctly:

[6,5,1,2,5,3,4,6,7,6,5] |
del(
  . as $dot |
  range(length) as $i |
  .[range($i + 1; length)] |
  select(. == $dot[$i])
)

output:

$ jq -cn '[6,5,1,2,5,3,4,6,7,6,5] | del(. as $dot | range(length) as $i | .[range($i + 1; length)] | select(. == $dot[$i]))'
[6,5,1,2,3,4,7]
$ jq -cn '[6,5,1,2,5,3,4,6,7,6,5] | delpaths([ path(. as $dot | range(length) as $i | .[range($i + 1; length)] | select(. == $dot[$i])) | debug ])'
["DEBUG:",[7]]
["DEBUG:",[9]]
["DEBUG:",[4]]
["DEBUG:",[10]]
["DEBUG:",[10]]
["DEBUG:",[9]]
[6,5,1,2,3,4,7]

I don't really understand this patch, so I am not sure what @capak07 had in mind with it. This does not solve the issue #2868 was created for, but maybe they are trying to solve a different bug?

emanuele6 avatar Feb 20 '24 03:02 emanuele6

I don't understand what you mean

emanuele6 avatar Feb 20 '24 18:02 emanuele6

Hmm, I am closing this PR since it seems the patch does not solve any issue, and otherwise it does not accomplish anything useful.

emanuele6 avatar Feb 29 '24 11:02 emanuele6

This is a tricky bug to fix.

nicowilliams avatar Mar 01 '24 21:03 nicowilliams