Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

select does not eliminate all mismatches #2422

Closed
davidBGraham66 opened this issue Mar 29, 2022 · 7 comments · Fixed by #2133
Closed

select does not eliminate all mismatches #2422

davidBGraham66 opened this issue Mar 29, 2022 · 7 comments · Fixed by #2133

Comments

@davidBGraham66
Copy link

the select function passes through objects that it should filter out

input:

{ "project":{ "channels":[
{ "cName":"c1", "cVal":"North" },
{ "cName":"c2", "cVal":"North" },
{ "cName":"c3", "cVal":"South" },
{ "cName":"c4", "cVal":"North" },
{ "cName":"c5", "cVal":"South" }
] } }

code:

.project.channels[] |= select( ."cVal" == "South")

expected ouput:
{
"project": {
"channels": [
{
"cName": "c3",
"cVal": "South"
},
{
"cName": "c5",
"cVal": "South"
}
]
}
}

actual output
{
"project": {
"channels": [
{
"cName": "c2",
"cVal": "North"
},
{
"cName": "c3",
"cVal": "South"
},
{
"cName": "c5",
"cVal": "South"
}
]
}
}

select failed to eliminate all objects where cVal was not "South"

The behavior seems to be that , if there are two objects in a row that need eliminated, it lets the second one pass .
This could happen if the code was traversing a linked list, testing each node, eliminating a node, then going to next node.
If the code eliminates a node, then the following node will become the current node, so the code should then test the (new) current node before moving on.

The behavior is seen on jqplay.org

(
and also on jq-win64.exe with proper windows quoting

jq-Win64.exe ".project.channels[] |= select( ."""cVal""" == """South""") " my.txt

)

@wader
Copy link
Member

wader commented Mar 29, 2022

Hi, I think you might be hitting the same issue as in #2051. A workaround could be something like:

.project.channels |= map(select(."cVal" == "South"))

@pkoppstein
Copy link
Contributor

pkoppstein commented Jun 23, 2023

The seemingly strange behavior of E |= F stems from the fact that its semantics is based on the paths defined by the LHS (E). An excellent discussion of this may be found at https://github.com/01mf02/jaq#assignments

This aspect of |= is one of the reasons (given the current behavior) it's perhaps better in general to prefer E |= map(select(...)) rather than E[] |= select(...), along the lines of @wader's suggestion.

(@itchyny - Perhaps the "bug" label should be changed? An ER for functionality? Documentation?)

@emanuele6
Copy link
Member

emanuele6 commented Jun 23, 2023

I suggest using del(.project.channels[] | select(.cVal != "South")) instead of .project.channels |= map(select(."cVal" == "South")).
map() will convert .project.channels to an array if it is an object, using del() makes the solution works correctly independently of whether .project.channels is an array or an object.

I don't think it ever makes sense to "prefer" |= map(select()) over del(.[] | select()) (or .[] |= select() if it worked as expected).
I also disagree that this should not be considered a bug, there is no reason to want the current behaviour, and I really doubt it was intentionally implemented the way it works now with arrays.

Both of jq's del/1 and delpaths/1 are able to delete elements correctly from arrays, only _modify/2 (a.k.a. |=) has this problem, and gojq implements it so that it works correctly when deleting elements from arrays.

@nicowilliams nicowilliams added bug and removed bug labels Jun 23, 2023
@nicowilliams
Copy link
Contributor

Ah, right, if you expect E |= select(P) to delete paths E whose values don't match the predicate P then you need jq from the master branch (IIRC I fixed this in master).

@pkoppstein
Copy link
Contributor

@nicowilliams wrote:

(IIRC I fixed this in master)

Apparently not, e.g.:

$ jqMaster -nM '{a: [1,2,3,3,2,1]}  | .a[] |= select(. != 2)'
{
  "a": [
    1,
    3,
    3,
    1,
    null,
    null
  ]
}
$ jqMaster --version
jq-1.6-201-gdd5ce98-dirty

@nicowilliams
Copy link
Contributor

(IIRC I fixed this in master)

Apparently not, e.g.:

Right. So the correct way to fix this (IMO) would be to add something like the SUBEXP_BEGIN/end opcodes but to signify that .[] should iterate arrays from the highest index to 0, and have the assignment operators cause that to be used. We might as well add syntax to allow one to do so oneself, something like .[-], and for symmetry (maybe maybe) .[+] to mean "always from index zero up", with .[] meaning "either from 0 if not in assignments, else from -1".

@nicowilliams
Copy link
Contributor

(IIRC I fixed this in master)

Apparently not, e.g.:

#2662

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants