zed icon indicating copy to clipboard operation
zed copied to clipboard

Can't assign named type to enum in ZSON

Open philrz opened this issue 3 years ago • 4 comments

Repro is with Zed commit c7f79e2b.

While doing a proofreading pass on the ZSON spec, I noticed that the Enum Value example isn't accepted as valid input.

$ zq -version
Version: v1.0.0-41-gc7f79e2b

$ cat enums.zson 
%HEADS (flip=(enum(HEADS,TAILS)))
%TAILS (flip)
%HEADS (flip)

$ zq -z enums.zson 
enums.zson: format detection error
	zeek: line 1: bad types/fields definition in zeek header
	zjson: line 1: invalid character '%' looking for beginning of value
	zson: parse error: type list not found parsing union type at '('
	zng: truncated input
	zng21: malformed zng record
	csv: record on line 2: wrong number of fields
	json: invalid character '%' looking for beginning of value
	parquet: auto-detection not supported
	zst: auto-detection not supported

I went hunting for a test that might show the correct syntax, but I couldn't find one. I could only make it work if I reiterated the type decorator each time, such as is done in the enum.yaml test.

$ cat enums-alt.zson 
%HEADS (enum(HEADS,TAILS))
%TAILS (enum(HEADS,TAILS))
%HEADS (enum(HEADS,TAILS))

$ zq -z enums-alt.zson 
%HEADS(enum(HEADS,TAILS))
%TAILS(enum(HEADS,TAILS))
%HEADS(enum(HEADS,TAILS))

philrz avatar Apr 19 '22 16:04 philrz

On closer inspection, it looks like there may be a more fundamental problem with named types, not just with enums. For instance, the section on Named Type claims the following is valid, but it's rejected:

$ echo '{p1:80 (port=(uint16)), p2: 8080 (port)}' | zq -z -
stdio:stdin: format detection error
	zeek: line 1: bad types/fields definition in zeek header
	zjson: line 1: invalid character 'p' looking for beginning of object key string
	zson: parse error: type list not found parsing union type at '('
	zng: unknown ZNG message frame type: 3
	zng21: malformed zng record
	csv: empty csv file
	json: invalid character 'p' looking for beginning of value
	parquet: auto-detection not supported
	zst: auto-detection not supported

Since I recalled named types for port still being created when reading Zeek TSV, I peeked at some ZSON output after reading those and confirmed that I can make it work if I drop the parens.

$ echo '{p1:80 (port=uint16), p2: 8080 (port)}' | zq -z -
{p1:80(port=uint16),p2:8080(port)}

However, this begs the question as to whether it's the spec or implementation that's incorrect here.

philrz avatar Apr 19 '22 17:04 philrz

@philrz I would say the spec is incorrect here.

mattnibs avatar Apr 26 '22 16:04 mattnibs

Per the most recent comment from @mattnibs, indeed, I bumped into that again and forgot this issue existed, so I've got #4086 up for review currently to update that general issue in the spec.

Applying what I learned there to the specific topic of enum types, dropping the parens gets me a different error message. So it seems an issue remains.

$ zq -version
Version: v1.2.0-49-g9c0f0973

$ cat enums-no-parens.zson 
%HEADS (flip=enum(HEADS,TAILS))
%TAILS (flip)
%HEADS (flip)

$ zq -i zson -z enums-no-parens.zson
enums-no-parens.zson: enum value is not of type enum

philrz avatar Sep 14 '22 19:09 philrz

When reviewing this one in a team discussion, @mccanne expressed that he's starting to have doubts whether Zed should even continue to offer this type of enum. He's agreed to own this issue to make a call on what's to be done with it.

philrz avatar Sep 21 '22 22:09 philrz