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

Add tiledb_field_get_nullable API #5378

Merged
merged 2 commits into from
Nov 20, 2024
Merged

Conversation

rroelke
Copy link
Contributor

@rroelke rroelke commented Nov 19, 2024

The set of "query field" APIs offers a means to ask about the datatype and cell val num of a query result field without understanding whether the queried field is a dimension, attribute, or aggregate.

For aggregates in particular tiledb_field_datatype and tiledb_field_cell_val_num allow a user to ask about the result type of their aggregate function operation, without having to read docs, source code, etc.

To date, however, there has not been a way to ask if a query field can be null. This pull request adds that, via a C API tiledb_field_get_nullable.


TYPE: C_API
DESC: Add tiledb_field_get_nullable API

int64_t d2[] = {1, 1, 1, 1, 1, 1, 1, 1, 1, 1};
uint64_t d1_size = 10 * sizeof(int64_t);
uint64_t d2_size = 10 * sizeof(int64_t);
int32_t a[] = {1, 2, 3, 4, 5, 6, 7, 8, 9};
Copy link
Contributor Author

@rroelke rroelke Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last cell doesn't make it through the write (one of the other inputs only has nine cells) so I elected to remove it to avoid future confusion.

(e.g. a developer expecting a sum of 55 rather than 45, you can probably guess who did that)

Copy link
Member

@teo-tsirpanis teo-tsirpanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, please add a C++ API.

Comment on lines +156 to +159
TILEDB_EXPORT capi_return_t tiledb_field_get_nullable(
tiledb_ctx_t* ctx,
tiledb_query_field_t* field,
uint8_t* nullable) TILEDB_NOEXCEPT;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call it is_nullable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that, in the end I chose to mimic tiledb_attribute_get_nullable

@rroelke
Copy link
Contributor Author

rroelke commented Nov 19, 2024

Looks good, please add a C++ API.

There is no analog in the C++ API for query field. They were introduced in #4385 which did not add anything in C++ and to date nothing has been added. I don't think this PR is the place to do so.

@teo-tsirpanis
Copy link
Member

Oh I didn't know query field has a C++ API, opened SC-59555 to track it.

I chose to mimic tiledb_attribute_get_nullable

OK, I saw some other APIs also not having is.

Copy link
Member

@teo-tsirpanis teo-tsirpanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@rroelke rroelke force-pushed the rr/sc-59534-tiledb_field_get_nullable branch from e71f424 to a128278 Compare November 19, 2024 17:49
@rroelke rroelke merged commit e6e57be into dev Nov 20, 2024
63 checks passed
@rroelke rroelke deleted the rr/sc-59534-tiledb_field_get_nullable branch November 20, 2024 01:59
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.

3 participants