Skip to content

Commit

Permalink
Only serialize Array URI if client side
Browse files Browse the repository at this point in the history
We should avoid returning the absolute array URI in the schema when
sending from server to client. Only serialize the array if we are client
side, in which case the URI was already known to open the array.
  • Loading branch information
Shelnutt2 committed Sep 22, 2020
1 parent 7c7c26e commit 60cf498
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 12 deletions.
1 change: 1 addition & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

* Fixed bug in setting a fill value for var-sized attributes.
* Fixed a bug where the cpp headers would always produce compile-time warnings about using the deprecated c-api "tiledb_coords()" [#1765](https://github.com/TileDB-Inc/TileDB/pull/1765)
* Only serialize the Array URI in the array schema client size. [#1806](https://github.com/TileDB-Inc/TileDB/pull/1806)

## API additions

Expand Down
6 changes: 2 additions & 4 deletions tiledb/sm/c_api/tiledb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4411,9 +4411,6 @@ int32_t tiledb_serialize_array_schema(
tiledb_serialization_type_t serialize_type,
int32_t client_side,
tiledb_buffer_t** buffer) {
// Currently unused:
(void)client_side;

// Sanity check
if (sanity_check(ctx) == TILEDB_ERR ||
sanity_check(ctx, array_schema) == TILEDB_ERR)
Expand All @@ -4429,7 +4426,8 @@ int32_t tiledb_serialize_array_schema(
tiledb::sm::serialization::array_schema_serialize(
array_schema->array_schema_,
(tiledb::sm::SerializationType)serialize_type,
(*buffer)->buffer_)))
(*buffer)->buffer_,
client_side)))
return TILEDB_ERR;

return TILEDB_OK;
Expand Down
2 changes: 1 addition & 1 deletion tiledb/sm/c_api/tiledb_serialization.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ TILEDB_EXPORT int32_t tiledb_serialization_type_from_str(
* @param ctx The TileDB context.
* @param array_schema The array schema to serialize.
* @param serialization_type Type of serialization to use
* @param client_side If set to 1, deserialize from "client-side" perspective.
* @param client_side If set to 1, serialize from "client-side" perspective.
* Else, "server-side."
* @param buffer Will be set to a newly allocated buffer containing the
* serialized max buffer sizes.
Expand Down
20 changes: 14 additions & 6 deletions tiledb/sm/serialization/array_schema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,12 +397,15 @@ Status domain_from_capnp(

Status array_schema_to_capnp(
const ArraySchema* array_schema,
capnp::ArraySchema::Builder* array_schema_builder) {
capnp::ArraySchema::Builder* array_schema_builder,
const bool client_side) {
if (array_schema == nullptr)
return LOG_STATUS(Status::SerializationError(
"Error serializing array schema; array schema is null."));

array_schema_builder->setUri(array_schema->array_uri().to_string());
// Only set the URI if client side
if (client_side)
array_schema_builder->setUri(array_schema->array_uri().to_string());
auto v = kj::heapArray<int32_t>(1);
v[0] = array_schema->version();
array_schema_builder->setVersion(v);
Expand Down Expand Up @@ -456,7 +459,9 @@ Status array_schema_from_capnp(
(*array_schema)->set_tile_order(layout);
RETURN_NOT_OK(layout_enum(schema_reader.getCellOrder().cStr(), &layout));

(*array_schema)->set_array_uri(URI(schema_reader.getUri().cStr()));
if (schema_reader.hasUri())
(*array_schema)->set_array_uri(URI(schema_reader.getUri().cStr()));

(*array_schema)->set_cell_order(layout);
(*array_schema)->set_capacity(schema_reader.getCapacity());
(*array_schema)->set_allows_dups(schema_reader.getAllowsDuplicates());
Expand Down Expand Up @@ -506,12 +511,14 @@ Status array_schema_from_capnp(
Status array_schema_serialize(
ArraySchema* array_schema,
SerializationType serialize_type,
Buffer* serialized_buffer) {
Buffer* serialized_buffer,
const bool client_side) {
try {
::capnp::MallocMessageBuilder message;
capnp::ArraySchema::Builder arraySchemaBuilder =
message.initRoot<capnp::ArraySchema>();
RETURN_NOT_OK(array_schema_to_capnp(array_schema, &arraySchemaBuilder));
RETURN_NOT_OK(
array_schema_to_capnp(array_schema, &arraySchemaBuilder, client_side));

serialized_buffer->reset_size();
serialized_buffer->reset_offset();
Expand Down Expand Up @@ -1372,7 +1379,8 @@ Status array_metadata_deserialize(

#else

Status array_schema_serialize(ArraySchema*, SerializationType, Buffer*) {
Status array_schema_serialize(
ArraySchema*, SerializationType, Buffer*, const bool) {
return LOG_STATUS(Status::SerializationError(
"Cannot serialize; serialization not enabled."));
}
Expand Down
12 changes: 11 additions & 1 deletion tiledb/sm/serialization/array_schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,20 @@ enum class SerializationType : uint8_t;

namespace serialization {

/**
* Serialize an array schema via Cap'n Prto
* @param array_schema schema object to serialize
* @param serialize_type format to serialize into Cap'n Proto or JSON
* @param serialized_buffer buffer to store serialized bytes in
* @param client_side indicate if client or server side. If server side we won't
* serialize the array URI
* @return
*/
Status array_schema_serialize(
ArraySchema* array_schema,
SerializationType serialize_type,
Buffer* serialized_buffer);
Buffer* serialized_buffer,
const bool client_side);

Status array_schema_deserialize(
ArraySchema** array_schema,
Expand Down

0 comments on commit 60cf498

Please sign in to comment.