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

Review and document edge cases related to inner-nullabillity #2993

Open
2 tasks
jleibs opened this issue Aug 15, 2023 · 0 comments
Open
2 tasks

Review and document edge cases related to inner-nullabillity #2993

jleibs opened this issue Aug 15, 2023 · 0 comments
Labels
😤 annoying Something in the UI / SDK is annoying to use codegen/idl

Comments

@jleibs
Copy link
Member

jleibs commented Aug 15, 2023

We have run into several cases where a ListArray or FixedSizedListArray ends up being nullable while its inner-types are non-nullable. Some of the codegen still ends up placing a validity-mask on these inner types, which can lead to discrependencies in the validation of our round-trip tests.

These are sort of unreachable nulls in a schema that says they shouldn't be nullable and so they fall into a grey area. On one hand you could argue they are inconsistent with the schema definition. On the other hand, the data that's in those inner arrays is undefined and so if something were to unwrap those inner arrays and look at them without the outer context, having nulls in there is possible the more correct thing to do.

In order to get the round-trip tests passing as part of #2991 we added this validity mask to the rust code.

Two pieces of work are associated with this issue:

  1. Review and make sure we are happy with the current state of affairs. As an alternative to the current approach we might instead consider making the C++ / Python code avoid adding these nulls or make the round-trip validation checks more lenient.
  2. We should add some more architectural primer/documentation on how Rerun interprets the intention and interactions between the schema definition and the validity masks.
@jleibs jleibs added 😤 annoying Something in the UI / SDK is annoying to use codegen/idl labels Aug 15, 2023
@jleibs jleibs modified the milestone: 0.8.1 Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😤 annoying Something in the UI / SDK is annoying to use codegen/idl
Projects
None yet
Development

No branches or pull requests

1 participant