brimdata / zed

A novel data lake based on super-structured data
https://zed.brimdata.io/
BSD 3-Clause "New" or "Revised" License
1.37k stars 67 forks source link

bug in scoped group-by expressions #3491

Open mccanne opened 2 years ago

mccanne commented 2 years ago

For this input...

{s:"a",x:[1,2]}
{s:"a",x:[3]}
{s:"b",x:[4,5]}
{s:"b",x:[6,7]}

and this Zed:

zq "let s=s over x => ( union(this) by s )" in.zson

or this Zed:

zq "let s=s over x => ( union(this) by s:=s )" in.zson

You get a compile error (I think the semantic analyzer doesn't handle converting the group-by expression correctly reasonably in the face of vars).

You can work around it with this...

zq "let s=s over x => ( union(this) by s2:=s )" in.zson

The problem could just be that lvals should not get var substitution.

philrz commented 2 weeks ago

As of current Zed commit 4360baf, this one seems to be fixed (or at least partially fixed).

The let operator was removed in #4531, but here's how the repro and workaround output was looking back at Zed commit 4f9950e around the time this issue was opened.

$ zq -version
Version: v0.33.0-108-g4f9950ea

$ zq "let s=s over x => ( union(this) by s )" in.zson
invalid expression on lhs of assignment

$ zq "let s=s over x => ( union(this) by s:=s )" in.zson
invalid expression on lhs of assignment

$ zq "let s=s over x => ( union(this) by s2:=s )" in.zson
{s2:"a",union:|[1,2]|}
{s2:"a",union:|[3]|}
{s2:"b",union:|[4,5]|}
{s2:"b",union:|[6,7]|}

It looks like the way to express the equivalent in current Zed (commit 4360baf at the moment) would be:

$ zq -version
Version: v1.17.0-37-g4360baf9

$ 'over x with s=s => (union(this) by s)' in.zson 
{s:"a",union:|[1,2]|}
{s:"a",union:|[3]|}
{s:"b",union:|[4,5]|}
{s:"b",union:|[6,7]|}

$ zq 'over x with s=s => (union(this) by s:=s)' in.zson 
illegal left-hand side of assignment at line 1, column 36:
over x with s=s => (union(this) by s:=s)
                                   ~~~~

As for that last one, the over docs explain:

Note that any such variable definitions override implied field references of this. If a both a field named "x" and a variable named "x" need be referenced in the lateral scope, the field reference should be qualified as this.x while the variable is referenced simply as x.

This leads me to assume the error message is actually providing correct guidance that the user could write it:

$ zq 'over x with s=s => (union(this) by this.s:=s)' in.zson
{s:"a",union:|[1,2]|}
{s:"a",union:|[3]|}
{s:"b",union:|[4,5]|}
{s:"b",union:|[6,7]|}

However, I'll hold this issue open and let Dev make the final call on if there's anything left to do here.