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

Inspector renders null and {} both as "null" #2454

Closed
philrz opened this issue Jul 20, 2022 · 6 comments · Fixed by #2875 or #2876
Closed

Inspector renders null and {} both as "null" #2454

philrz opened this issue Jul 20, 2022 · 6 comments · Fixed by #2875 or #2876
Assignees
Labels
bug Something isn't working

Comments

@philrz
Copy link
Contributor

philrz commented Jul 20, 2022

Repro is with Brim commit 55d1fdc.

Consider the following test data empties.ndjson:

{"empty_object": {}}
{"null_value": null}

When output by Zed CLI tooling, the different nature of the two values is preserved.

$ zq -version
Version: v1.2.0-7-g39539336

$ zq -Z empties.ndjson 
{
    empty_object: {}
}
{
    null_value: null
}

However, when rendered in the Inspector, both values end up looking the same.

image

@philrz philrz added the bug Something isn't working label Jul 20, 2022
@philrz
Copy link
Contributor Author

philrz commented Jul 20, 2022

Also, I'm not sure if this is related, but the attached video shows no hover text in the Table view when rendering the empty_object. Like in #2453, I'm not certain if the Table view should just bail entirely on trying to render any of this.

Repro.mp4

When the primary issue here gets fixed, if this remains as a problem, I'll open a separate issue.

@jameskerr
Copy link
Member

This is great. Thanks for tracking this.

@philrz
Copy link
Contributor Author

philrz commented Feb 3, 2023

I happened to encounter this issue again and rechecked how things are looking now in the era of the redesigned Table at Brim commit 1e0a608.

As it shows, when shown as multiple shapes, the test data from empties.ndjson shows both vales as null, which is unsurprising since the Table view has taken on some behaviors of the Inspector view in this multi-shape context, so that bug remains. A separate new-ish behavior shown is when I do head 1 | yield empty_object to output just the {} value the table view is effectively completely blank, i.e., there's not even a this column header shown like when we do tail 1 | yield null_value.

Repro.mp4

@philrz
Copy link
Contributor Author

philrz commented Oct 4, 2023

I was testing the Preview & Load branch, running through the steps in the zq tutorial using the new workflow. This reminded me how this issue is still with us, since the output given here by the CLI tooling is {} (which is what would be the correct output, not null). I'm going to push this issue into the backlog in hopes we can address it before the next GA release.

image

@philrz philrz linked a pull request Nov 2, 2023 that will close this issue
@philrz
Copy link
Contributor Author

philrz commented Nov 2, 2023

The changes in linked PR #2875 thankfully have addressed this for the specific example shown above. Repeating the repro steps at Zui commit b90a400, we now see:

image

However, when I opened this issue I mistakenly thought it was something general to handling of null vs. {} in all contexts as opposed to on a per-complex-Zed-type basis. This led me to check the other types and indeed it looks like the same exposure exists elsewhere. This input data shows values inside an array, a set, a map, and also the top-level primitive values.

$ zq -Z empties2.ndjson 
{
    r: [
        {},
        null ({})
    ]
}
{
    s: |[
        null ({}),
        {}
    ]|
}
{
    m: |{
        "APPL": null ({}),
        "GOOG": {}
    }|
}
{}
null

Rendered in Zui at commit b90a400, we see the problem does indeed remain for the other complex types, though the top-level primitive ones look fine.

image

@philrz philrz mentioned this issue Nov 7, 2023
@philrz
Copy link
Contributor Author

philrz commented Nov 7, 2023

Verified in Zui commit d29e674.

The second set of values highlighted in the comment above are now being rendered similar to ZSON.

image

A wider discussion about possible divergence between ZSON presentation and Zui's ZSON-like rendering similar to what's been tracked in this issue has led to the opening of #2880 to track a more automated approach we may use one day to sniff out other not-yet-found examples of problems like this and keep us from regressing.

Thanks @jameskerr!

@philrz philrz closed this as completed Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants