Skip to content

jq: recurse's f is not suspended between its own outputs, so a node's later f outputs still fire (#2693 residual) #2918

Description

@newhoggy

Severity: Low (side-effect count and stderr ordering; delivered values are correct)

Summary

#2693 stopped recurse from
evaluating f at a node the consumer was already satisfied by — limit(1; recurse(f)) went
from running f at 10000 nodes to running it at none. What it did not change: at the node
the traversal is at, f runs to completion before any of its outputs is descended into.

jq's def r: ., (f | r); r; interleaves instead: the first output of f has its whole subtree
traversed before f is asked for its second. So jq stops between f's outputs, and
succinctly cannot.

Repro

Confirmed live against jq 1.7.1 and succinctly at 81209f1ab (post-#2693). Values match in
every row; only stderr differs.

Under a bound — one node's fan-out too many (it was the whole remaining tree before #2693):

$ echo '[1,[2,[3]]]' | jq -c '[limit(2; recurse(.[]?|debug))]'
["DEBUG:",1]                                  # succinctly: ["DEBUG:",1] ["DEBUG:",[2,[3]]]

Unbounded, it shows as stderr ordering, because a node's whole fan-out is evaluated before
its first child is descended into:

$ echo '{"a":{"x":1},"b":{"y":2}}' | jq -c '[recurse(.[]?|debug)]'
["DEBUG:",{"x":1}] ["DEBUG:",1] ["DEBUG:",{"y":2}] ["DEBUG:",2]
# succinctly:
["DEBUG:",{"x":1}] ["DEBUG:",{"y":2}] ["DEBUG:",1] ["DEBUG:",2]

And once through ?, an extra f run at the node that errors:

$ echo '{"a":1}' | jq -c '[recurse(.[]?, ("x"|stderr|error("f")))?]'
x                                             # succinctly: xx

Both evaluators are affected identically (each_recurse_walk is shared), and
resolve_recurse_sink has the same residual on the path side —
path(limit(2; recurse((.a|debug), (.b|debug)))) on {"a":1,"b":2} writes one DEBUG line in
jq and two here.

Why it is not a small fix

The traversal is an explicit LIFO stack of materialized nodes, in all three walkers. To stop
between f's outputs the stack would have to hold f's suspended generator at each level,
and f is driven by a push-based sink that cannot be resumed. The natural expression is
recursion inside f's own sink callback — child N's subtree runs before f yields child N+1 —
but that makes native stack depth equal to traversal depth, and RECURSE_MAX_ITEMS allows
10000. This repo has measured numbers for that hazard: MAX_EXPR_DEPTH's note records the
parser — a much smaller frame — aborting at around 96 levels in debug on cargo's 2 MiB test
thread.

So the options are materially different and none is obviously right:

  1. Bounded native recursion with a fallback. Recurse in the callback while depth ≤ D, and
    use today's collecting walk past D. Exact for every realistic shape (a fan-out deeper than
    ~13 exceeds RECURSE_MAX_ITEMS anyway), but adds a second traversal shape to each of the
    three walkers — each of which carries jq: recurse/recurse(f) drop a null child and flatten a single array-valued child #490/jq: recurse(f)/recurse(f;cond) use breadth-first queue order, diverging from jq's depth-first definition #635/jq: recurse(f; cond) silently prunes on a cond/f error instead of propagating it like jq does #636/jq: recurse(f) drops f's own partial fan-out when a later f output errors (value + path evaluators) #842/jq: recurse(f; cond) drops cond's already-approved siblings on a later cond error #854's rules — which is the
    "duplicated predicates diverge silently" hazard CLAUDE.md names.
  2. A pull-based generator protocol for the evaluator/resolver, so a generator can be
    suspended rather than only stopped. This closes the whole class at once — this issue,
    jq: path()'s own consumer side still collects, so a bound outside path() can't stop the fold (#2694 residual) #2908, and resolve_recurse_sink's own half — but is architecture-level (new ADR,
    invasive).
  3. Leave it. The values are right; the cost is a bounded side-effect over-fire and stderr
    ordering.

Filing rather than choosing, since option 2's scope is a different tier from this issue's.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    OpusSuitable for an Opus-class model to implement

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions