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

Panic when using "over" in an expression context #5077

Closed
philrz opened this issue Mar 16, 2024 · 1 comment · Fixed by #5079
Closed

Panic when using "over" in an expression context #5077

philrz opened this issue Mar 16, 2024 · 1 comment · Fixed by #5079
Assignees
Labels
bug Something isn't working community

Comments

@philrz
Copy link
Contributor

philrz commented Mar 16, 2024

Repro is with Zed commit f1be6a4.

@mattnibs and I were working toward responding to a community user's inquiry in a Slack thread. I'd gotten to the point where I had this working in a user-defined operator:

$ cat op.zed 
op MyFlatten(val): (
  yield (over val | over this | collect(this))

$ zq -version
Version: v1.14.0-17-gf1be6a4a

$ echo '[[1,2],[3,4]]' | zq -I op.zed 'MyFlatten(this)' -
[1,2,3,4]

So I thought to try the equivalent in a user-defined function, which triggered this panic.

$ cat func.zed 
func MyFlatten(val): (
  (over val | over this | collect(this))
)

$ echo '[[1,2],[3,4]]' | zq -I func.zed 'yield MyFlatten(this)' -
panic: runtime error: index out of range [0] with length 0

goroutine 53 [running]:
github.com/brimdata/zed/runtime/sam/expr.Var.Eval(...)
	/Users/phil/work/zed/runtime/sam/expr/var.go:14
github.com/brimdata/zed/runtime/sam/op/traverse.(*Over).over(0xc0003e3da0, {0x28d5478?, 0xc000789f50}, {{0x28c76d0?, 0x36de9e0?}, 0x0?, 0x0?})
	/Users/phil/work/zed/runtime/sam/op/traverse/over.go:72 +0x164
github.com/brimdata/zed/runtime/sam/op/traverse.(*Over).Pull(0xc0003e3da0, 0xc0?)
	/Users/phil/work/zed/runtime/sam/op/traverse/over.go:56 +0x1a9
github.com/brimdata/zed/runtime/sam/op/traverse.(*Scope).run(0xc0006d1c70)
	/Users/phil/work/zed/runtime/sam/op/traverse/scope.go:69 +0x37
created by github.com/brimdata/zed/runtime/sam/op/traverse.(*Scope).Pull.func1 in goroutine 52
	/Users/phil/work/zed/runtime/sam/op/traverse/scope.go:47 +0x56

Meanwhile @mattnibs was coming up with a more sophisticated variant that was closer to the user's original intent, and it looks like he hit a similar panic.

$ cat maybe_flatten.zed
func maybe_flatten_fn(a): (
   kind(a) == "array"  
     ? (over a | and(kind(this) == "array")) 
        ? "It's an array of arrays, so I would flatten"
        : "It's an array containing some non-array values, so I would not flatten"
     : "It's not even an array, so I ain't touchin' that"
)

yield [[1,2],[3,4]], "hello"
| yield maybe_flatten_fn(this)

$ echo null | zq -I maybe_flatten.zed -
panic: runtime error: index out of range [0] with length 0

goroutine 25 [running]:
github.com/brimdata/zed/runtime/sam/expr.Var.Eval(...)
	/Users/phil/work/zed/runtime/sam/expr/var.go:14
github.com/brimdata/zed/runtime/sam/op/traverse.(*Over).over(0xc00056e840, {0x28d5478?, 0xc000876e10}, {{0x28c76d0?, 0x36de9e0?}, 0x0?, 0x0?})
	/Users/phil/work/zed/runtime/sam/op/traverse/over.go:72 +0x164
github.com/brimdata/zed/runtime/sam/op/traverse.(*Over).Pull(0xc00056e840, 0x0?)
	/Users/phil/work/zed/runtime/sam/op/traverse/over.go:56 +0x1a9
github.com/brimdata/zed/runtime/sam/op/traverse.(*Scope).run(0xc0006788c0)
	/Users/phil/work/zed/runtime/sam/op/traverse/scope.go:69 +0x37
created by github.com/brimdata/zed/runtime/sam/op/traverse.(*Scope).Pull.func1 in goroutine 24
	/Users/phil/work/zed/runtime/sam/op/traverse/scope.go:47 +0x56

As a side-effect of working on this, we also both recognized that this functionality should be mentioned in the expressions docs, but we might as well fix the bug before calling more attention to it.

@philrz philrz added bug Something isn't working community labels Mar 16, 2024
@mattnibs mattnibs self-assigned this Mar 18, 2024
mattnibs added a commit that referenced this issue Mar 18, 2024
This commit fixes a bug in over expressions where vars where outer
variables where not getting propagated to batches in the inner scope.

Closes #5077
mattnibs added a commit that referenced this issue Mar 18, 2024
This commit fixes a bug in over expressions where vars where outer
variables where not getting propagated to batches in the inner scope.

Closes #5077
mattnibs added a commit that referenced this issue Mar 18, 2024
This commit fixes a bug in over expressions where vars where outer
variables where not getting propagated to batches in the inner scope.

Closes #5077
mattnibs added a commit that referenced this issue Mar 18, 2024
This commit fixes a bug in over expressions where outer scope variables
were not getting propagated to batches in the inner scope.

Closes #5077
mattnibs added a commit that referenced this issue Mar 19, 2024
This commit fixes a bug in over expressions where outer scope variables
were not getting propagated to batches in the inner scope.

Closes #5077
@philrz
Copy link
Contributor Author

philrz commented Mar 19, 2024

Verified in Zed commit 3e5febe.

Both of the programs shown above that previously caused a panic now run successfully.

$ zq -version
Version: v1.14.0-21-g3e5febe7

$ echo '[[1,2],[3,4]]' | zq -I func.zed 'yield MyFlatten(this)' -
[1,2,3,4]

$ echo null | zq -I maybe_flatten.zed -
"It's an array of arrays, so I would flatten"
"It's not even an array, so I ain't touchin' that"

Thanks @mattnibs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants