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 stateful expressions in UDFs #5093

Merged
merged 2 commits into from
Jun 19, 2024
Merged

Fix stateful expressions in UDFs #5093

merged 2 commits into from
Jun 19, 2024

Conversation

mattnibs
Copy link
Collaborator

This commit fixes an issue with using aggregation expressions user-defined functions where there wasn't a separate state per textual invocation.

Closes #5092

@mattnibs mattnibs requested a review from a team March 27, 2024 18:32
@mattnibs mattnibs force-pushed the udf-stateful-expr branch from fbef121 to e6e84dc Compare March 27, 2024 18:37
@nwt nwt force-pushed the reset-stateful branch from d756794 to b35eb4f Compare June 6, 2024 14:56
@nwt nwt force-pushed the udf-stateful-expr branch from e6e84dc to 1864522 Compare June 6, 2024 15:11
Base automatically changed from reset-stateful to main June 7, 2024 16:50
This commit fixes an issue with using aggregation expressions user-defined
functions where there wasn't a separate state per textual invocation.

Closes #5092
@mattnibs mattnibs force-pushed the udf-stateful-expr branch from 1864522 to d3269a2 Compare June 7, 2024 16:56
compiler/kernel/expr.go Outdated Show resolved Hide resolved
Copy link
Member

@nwt nwt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these two programs produce different output by design?

$ echo 0 1 0 1 | zq '
  func f(x): (
    x == 0 ? count() : count()
  )

  yield f(this)
' -
1(uint64)
1(uint64)
2(uint64)
2(uint64)
$ echo 0 1 0 1 | zq '
  func my_count(): (
    count()
  )

  func f(x): (
    x == 0 ? my_count() : my_count()
  )

  yield f(this)
' -
1(uint64)
2(uint64)
3(uint64)
4(uint64)

compiler/kernel/op.go Outdated Show resolved Hide resolved
@mattnibs
Copy link
Collaborator Author

mattnibs commented Jun 17, 2024

Do these two programs produce different output by design?

This seems like a bug to me. Looking into it.

EDIT

Good catch, I think this highlights a limitation of my approach. Going to have to think about this one.

@mattnibs mattnibs requested a review from nwt June 19, 2024 17:47
@mattnibs mattnibs merged commit 9c3f5fd into main Jun 19, 2024
3 checks passed
@mattnibs mattnibs deleted the udf-stateful-expr branch June 19, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stateful expressions in user defined functions
2 participants