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

fix: reduce/foreach state variable should not be reset each iteration #3205

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Nov 15, 2024

When the UPDATE query in a reduce or foreach syntax does not emit
a value, it should simply skip updating the state variable. But
currently it resets the state variable to null.

When the UPDATE query in a reduce or foreach syntax does not emit
a value, it should simply skip updating the state variable. But
currently it resets the state variable to null.
@itchyny itchyny added this to the 1.8 release milestone Nov 15, 2024
@itchyny itchyny added the bug label Nov 15, 2024
@itchyny
Copy link
Contributor Author

itchyny commented Nov 16, 2024

Now these syntax behaves like the following pseudo code.
Previously, $x was assigned null when it is used after ($x|UPDATE).

# reduce EXP as $v (INIT; UPDATE)
for $x in INIT {
  for $v in EXP {
    for $u in ($x|UPDATE) {
      $x = $u
    }
  }
  yield $x
}

# foreach EXP as $v (INIT; UPDATE; EXTRACT)
for $x in INIT {
  for $v in EXP {
    for $u in ($x|UPDATE) {
      $x = $u
      for $e in ($x|EXTRACT) {
        yield $e
      }
    }
  }
}

These procedural codes explain any cases the filters emit multiple or no values.
The EXTRACT filter is evaluated against each UPDATE output, and the last value is used for
the next iteration. And if UPDATE emits no value, it skips updating the intermediate value.
I wondered whether we should change the behavior to how jaq does (multiple values
in UPDATE replicates successive iteration, resulting into exponentially many results,
and no values aborts the entire loop). But I now think the current behavior
(after this PR fix about the intermediate value reset) looks well-defined.
Although the jq manual does not explains well about these cases,
the manual shouldn't be too much detailed about such minor use cases.
I think the current example codes makes it easy to understand these syntaxes,
especially for beginners.
Also, I noticed a slight difference in gojq's reduce syntax about INIT, I'll fix this soon.

@wader
Copy link
Member

wader commented Nov 16, 2024

Update docs in this PR?

@itchyny
Copy link
Contributor Author

itchyny commented Nov 16, 2024

I'd like to make this PR focus on the null reset. The change is too detailed to write in the current user manual in my opinion.

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

Successfully merging this pull request may close these issues.

3 participants