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

ER: simpler, faster walk/1 #2611

Closed
pkoppstein opened this issue Jun 13, 2023 · 0 comments · Fixed by #2795
Closed

ER: simpler, faster walk/1 #2611

pkoppstein opened this issue Jun 13, 2023 · 0 comments · Fixed by #2795

Comments

@pkoppstein
Copy link
Contributor

A well-known efficiency enhancement for walk/1 is to use an inner function along the lines of:

def walk(f):
  def w: ...;
  w;

In addition to this efficiency enhancement, I would like to propose a very small change
that both greatly simplifies the definition and resolves some other issues, notably a discrepancy with the semantics of
assignments of the form .b = empty.

For example, the expression:

[ { a: {b: 2, c: 3} } ] | map(.b = empty)

yields []

so one would expect (and perhaps require) that:

walk(if type == "object" then .b = empty]

should also yield []. Using the current def of walk/1, however, it yields [null] instead.

Here then is the proposed new version of walk/1, which is
faster, simpler and more conformant as described above:

# Apply f to composite entities recursively, and to atoms
def walk(f):
  def w:
    if type == "object"
    then map_values(w)
    elif type == "array" then map(w)
    else .
    end
    | f;
  w;

Please also note that the above proposed definition is sufficient to resolve the issue raised in #2584.

@pkoppstein pkoppstein added the bug label Jul 8, 2023
pkoppstein added a commit to pkoppstein/jq that referenced this issue Jul 31, 2023
pkoppstein added a commit to pkoppstein/jq that referenced this issue Jul 31, 2023
commit 9afda0a
Merge: 4665e81 eeb08b5
Author: pkoppstein <pkoppstein@gmail.com>
Date:   Mon Jul 31 07:33:58 2023 -0400

    Merge branch 'walk' of https://github.com/pkoppstein/jq into walk

commit 4665e81
Author: pkoppstein <pkoppstein@gmail.com>
Date:   Mon Jul 31 07:26:03 2023 -0400

    revamp walk/1: rebase, add test cases

    Note that according to the new def:

    `{x:0} | walk(.,1)` is equivalent to: {x:0} | walk(.), walk(1)

commit bdec9c0
Author: pkoppstein <pkoppstein@gmail.com>
Date:   Wed Jul 5 02:00:59 2023 -0400

    revamp walk/1

    Resolves jqlang#2584; see also jqlang#2611

commit c8e28da
Author: itchyny <itchyny@cybozu.co.jp>
Date:   Mon Jul 31 09:52:52 2023 +0900

    Redesign website (jqlang#2628)

    * Bump up Bootstrap to v5.3.1, Bootstrap Icon to v1.10.5.
    * Use autoComplete.js to drop dependency on jQuery and typeahead.js.
    * Support dark mode.
    * New svg logo and icon with responsive color mode support.
    * Normalize section ids to lower kebab-case for easiness of linking.
    * Use relative paths for links for local development (--root /output).
    * Various markup cleanups and accessibility improvements.

commit 4af3f99
Author: Owen Ou <o@owenou.com>
Date:   Sat Jul 29 07:20:48 2023 -0700

    Update `bug_report.md` with Discord link

commit 82f7f77
Author: Owen Ou <o@owenou.com>
Date:   Sat Jul 29 07:15:57 2023 -0700

    Redirect questions to Discord

    We now have an official Discord server and most maintainers are hanging
    out there. It would be a good idea to redirect questions to Discord.

commit f733a15
Author: Nicolas Williams <nico@cryptonector.com>
Date:   Mon Jul 10 18:29:03 2023 -0500

    Use -Wno-cast-function-type to quiet many warnings

commit c8b30df
Author: Nicolas Williams <nico@cryptonector.com>
Date:   Mon Jul 10 18:28:33 2023 -0500

    Add JQ_FALLTHROUGH and use it to quiet warnings

commit a6eb055
Author: Emanuele Torre <torreemanuele6@gmail.com>
Date:   Sat Jul 29 21:57:40 2023 +0200

    Fix typo in manual: "-seq" => "--seq"

commit ee2a215
Author: Owen Ou <169064+owenthereal@users.noreply.github.com>
Date:   Sat Jul 29 07:38:08 2023 -0700

    Backfill with references in NEWS.md (jqlang#2788)

    Backfill with references to PRs & issues in NEWS.md

commit eeb08b5
Author: pkoppstein <pkoppstein@gmail.com>
Date:   Wed Jul 5 02:00:59 2023 -0400

    revamp walk/1

    Resolves jqlang#2584; see also jqlang#2611
pkoppstein added a commit to pkoppstein/jq that referenced this issue Jul 31, 2023
Resolves jqlang#2584; also resolves jqlang#2611
and supersedes jqlang#2655

Note that according to the revised implementation:

`{x:0} | walk(.,1)` is equivalent to `{x:0} | walk(.), walk(1)`
@itchyny itchyny added this to the 1.7 release milestone Jul 31, 2023
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.

2 participants