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 More Nulls #2876

Merged
merged 1 commit into from
Nov 6, 2023
Merged

Fix More Nulls #2876

merged 1 commit into from
Nov 6, 2023

Conversation

jameskerr
Copy link
Member

@jameskerr jameskerr commented Nov 2, 2023

  • Records can have null fields.
  • Fix More Nulls

That empties file now looks like this:

CleanShot 2023-11-02 at 16 38 47@2x

Closes #2454

@philrz
Copy link
Contributor

philrz commented Nov 3, 2023

I clicked Approve because this indeed fixed all the examples shown above. However, there was a yet another variation I floated yesterday to @nwt that I now see is still divergent here from how ZSON shows it.

$ echo '{"r": [null,{},1]}' | zq -Z -
{
   r: [
       null,
       {},
       1
   ]
}

This branch at commit 3726e3d still shows an explicit decorator here.

image

I discussed this with @nwt and he pointed out that having an explicit decorator in this case is not wrong necessarily, since the Type Decorators part of the ZSON spec points out where they "might be required" with unions to remove ambiguity, but that does not preclude an implementation from choosing to be explicit and include them even if there's no ambiguity.

That said, I'm still not sure if there's a problem here. It's not like I can cut & paste the output as-is and treat it as ZSON since there's additional decoration unique to the app (e.g., the array indexes like 0:). But if I drop those and keep the rest, and put the ZSON-ish value from app next to the original ZSON value, I get different typeof() resuls.

$ cat funky_r.zson 
{r: [null ((int64,{})), {}, 1 ]}
{r: [null,{},1]}

$ zq -z 'typeof(r)' funky_r.zson 
<[(int64,{},(int64,{}))]>
<[(int64,{})]>

It looks like it's showing a union inside of a union. @nwt: Is that something that should be addressed?

@jameskerr jameskerr merged commit 4186053 into main Nov 6, 2023
3 checks passed
@jameskerr jameskerr deleted the fix-nulls branch November 6, 2023 17:57
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.

Inspector renders null and {} both as "null"
2 participants