Skip to content
New issue

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

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

Already on GitHub? # to your account

paths(scalars) bug? #1163

Closed
alex-ozdemir opened this issue Jun 13, 2016 · 12 comments · Fixed by #2666
Closed

paths(scalars) bug? #1163

alex-ozdemir opened this issue Jun 13, 2016 · 12 comments · Fixed by #2666

Comments

@alex-ozdemir
Copy link
Contributor

Consider this JSON:

{
  "size": 2,
  "unsaf": false
}

The instructions paths(scalars) yield just

[
  [
    "size"
  ]
]

Whereas one might expect it should yield

[
  [
    "size"
  ],
  [
    "unsaf"
  ]
]

A bit of poking around makes me think the problem is when false is a value in a object.

@alex-ozdemir
Copy link
Contributor Author

Oh, hmm - paths is just getting confused by the output of scalars. Thats unfortunate, but perhaps not a bug.
It does make the leaf_paths alias somewhat unfortunate though.

@ghost
Copy link

ghost commented Jun 21, 2016

This is almost surely a bug.

@alex-ozdemir
Copy link
Contributor Author

alex-ozdemir commented Jul 6, 2016

Okay, I submitted a PR (#1178) which fixes the issue with false leaves.
The same issue exists with null leaves, but it will be harder to fix (paths is what is messing us up there).

EDIT: PR now fixes the problem with null as well.

@pkoppstein
Copy link
Contributor

While we await the incorporation of a fix into "master", here is a stand-alone definition of paths/1 based on @alex-ozdemir's contribution:

def paths:
  def conditional_recurse(f):  def r: ., (select(.!=null) | f | r); r;
  path(conditional_recurse(.[]?)) | select(length > 0);

This definition is written so that anyone who wants to define conditional_recurse/1 as a top-level filter can easily do so. For reference:

  • the builtin recurse(f) is defined in terms of: def r: (f | select | r);
  • conditional_recurse(f) is defined in terms of: def r: (select | f | r);

@saxonww
Copy link

saxonww commented Dec 14, 2017

Came to report this same issue. I'm surprised it's been sitting for 18 months - is there a reason #1178 can't be accepted?

Another workaround may be e.g. path(..|select(type!="array" and type!="object"))

@alex-ozdemir
Copy link
Contributor Author

What a blast from the past.

I believe we're just waiting on someone with write access to the repo...

@saxonww
Copy link

saxonww commented Dec 15, 2017

Hello @nicowilliams, I see you are the most frequent committer, can you recommend the best way to get a PR noticed?

@nicowilliams
Copy link
Contributor

nicowilliams commented Dec 15, 2017

I'll review this next week.

EDIT: Swypo.

@ajasmin
Copy link
Contributor

ajasmin commented Dec 14, 2020

Until this is fixed a workaround is to check if scalars returns some non-empty value

paths([scalars] != [])

@nicowilliams
Copy link
Contributor

nicowilliams commented Aug 11, 2023

This is quite mysterious:

@nicowilliams nicowilliams reopened this Aug 11, 2023
@pkoppstein
Copy link
Contributor

@nicowilliams wrote:

This is quite mysterious:

All three results are correct. You can verify this e.g. by using gojq.

@nicowilliams
Copy link
Contributor

nicowilliams commented Aug 11, 2023

@pkoppstein Ah, I'm confused. The issue is a user error, sort of. You want: paths(scalars|true), not paths(scalars).

The issue is that paths/1 wants its argument to be a boolean predicate, but scalars isn't a boolean predicate -- it's a filter that passes through only scalar values, and false is a scalar value, but false makes paths/1 not emit the path.

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

Successfully merging a pull request may close this issue.

6 participants