Skip to content

Commit

Permalink
Array schema: C.41 changes, test support. (TileDB-Inc#4004)
Browse files Browse the repository at this point in the history 
Create new classes for setting up array schema objects for unit tests on the inside of the API. They use `std::initializer_list` for compact specification of schemas. Write new unit tests about coherence of the attribute containers inside `class ArraySchema`. Convert existing `array_schema` unit tests to use new support classes.

Added `std::initializer_list` constructors for `class ByteVecValue` and `class Range`. These are used within the new test support classes. Update some definitions in `types.h`.

Clean up `class ArraySchema`. Remove dead code. Change return of `ArraySchema::check` from `Status` to `void`. Adjust declarations.

Remove `ArraySchema::init()`, which was essentially dead code. Remove `Domain::init()`, which lost its last remaining use.

---
TYPE: IMPROVEMENT
DESC: Array schema: C.41 changes, test support.
  • Loading branch information
@eric-hughes-tiledb @davisp
eric-hughes-tiledb authored and davisp committed Sep 6, 2023
1 parent 50fc5db commit 713e733
Show file tree
Hide file tree
Showing 14 changed files with 620 additions and 420 deletions.
2 changes: 0 additions & 2 deletions test/src/unit-filter-pipeline.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1419,7 +1419,6 @@ TEST_CASE("Filter: Test compression", "[filter][compression]") {
CHECK(schema.add_attribute(make_shared<tiledb::sm::Attribute>(HERE(), attr))
.ok());
CHECK(schema.set_domain(domain).ok());
CHECK(schema.init().ok());

FilterPipeline pipeline;
ThreadPool tp(4);
Expand Down Expand Up @@ -1545,7 +1544,6 @@ TEST_CASE("Filter: Test compression var", "[filter][compression][var]") {
CHECK(schema.add_attribute(make_shared<tiledb::sm::Attribute>(HERE(), attr))
.ok());
CHECK(schema.set_domain(domain).ok());
CHECK(schema.init().ok());

FilterPipeline pipeline;
ThreadPool tp(4);
Expand Down
67 changes: 23 additions & 44 deletions tiledb/sm/array_schema/array_schema.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@

using namespace tiledb::common;

namespace tiledb {
namespace sm {
namespace tiledb::sm {

/** Class for locally generated status exceptions. */
class ArraySchemaException : public StatusException {
Expand Down Expand Up @@ -202,8 +201,8 @@ ArraySchema::ArraySchema(
}

// Create attribute map
auto n{static_cast<unsigned int>(attributes_.size())};
for (unsigned int i = 0; i < n; ++i) {
auto n{attribute_num()};
for (decltype(n) i = 0; i < n; ++i) {
auto attr = attributes_[i].get();
attribute_map_[attr->name()] = {attr, i};
}
Expand Down Expand Up @@ -311,10 +310,6 @@ shared_ptr<const Attribute> ArraySchema::shared_attribute(
return attributes_[it->second.index];
}

ArraySchema::attribute_size_type ArraySchema::attribute_num() const {
return static_cast<attribute_size_type>(attributes_.size());
}

const std::vector<shared_ptr<const Attribute>>& ArraySchema::attributes()
const {
return attributes_;
Expand Down Expand Up @@ -455,53 +450,52 @@ void ArraySchema::check_webp_filter() const {
}
}

Status ArraySchema::check() const {
void ArraySchema::check() const {
if (domain_ == nullptr)
return LOG_STATUS(
Status_ArraySchemaError("Array schema check failed; Domain not set"));
throw ArraySchemaException{"Array schema check failed; Domain not set"};

auto dim_num = this->dim_num();
if (dim_num == 0)
return LOG_STATUS(Status_ArraySchemaError(
"Array schema check failed; No dimensions provided"));
throw ArraySchemaException{
"Array schema check failed; No dimensions provided"};

if (cell_order_ == Layout::HILBERT && dim_num > Hilbert::HC_MAX_DIM) {
return LOG_STATUS(Status_ArraySchemaError(
throw ArraySchemaException{
"Array schema check failed; Maximum dimensions supported by Hilbert "
"order exceeded"));
"order exceeded"};
}

if (array_type_ == ArrayType::DENSE) {
auto type{domain_->dimension_ptr(0)->type()};
if (datatype_is_real(type)) {
return LOG_STATUS(
Status_ArraySchemaError("Array schema check failed; Dense arrays "
"cannot have floating point domains"));
throw ArraySchemaException{
"Array schema check failed; Dense arrays "
"cannot have floating point domains"};
}
if (attributes_.size() == 0) {
return LOG_STATUS(Status_ArraySchemaError(
"Array schema check failed; No attributes provided"));
throw ArraySchemaException{
"Array schema check failed; No attributes provided"};
}
}

if (array_type_ == ArrayType::SPARSE && capacity_ == 0) {
throw ArraySchemaException(
throw ArraySchemaException{
"Array schema check failed; Sparse arrays "
"cannot have their capacity equal to zero.");
"cannot have their capacity equal to zero."};
}

RETURN_NOT_OK(check_double_delta_compressor(coords_filters()));
RETURN_NOT_OK(check_string_compressor(coords_filters()));
throw_if_not_ok(check_double_delta_compressor(coords_filters()));
throw_if_not_ok(check_string_compressor(coords_filters()));
check_attribute_dimension_label_names();
check_webp_filter();

// Check for ordered attributes on sparse arrays and arrays with more than one
// dimension.
if (array_type_ == ArrayType::SPARSE || this->dim_num() != 1) {
if (has_ordered_attributes()) {
throw ArraySchemaException(
throw ArraySchemaException{
"Array schema check failed; Ordered attributes are only supported on "
"dense arrays with 1 dimension.");
"dense arrays with 1 dimension."};
}
}

Expand All @@ -513,17 +507,14 @@ Status ArraySchema::check() const {
for (auto label : dimension_labels_) {
if (!label->is_external()) {
if (!label->has_schema()) {
return LOG_STATUS(Status_ArraySchemaError(
throw ArraySchemaException{
"Array schema check failed; Missing dimension label schema for "
"dimension label '" +
label->name() + "'."));
label->name() + "'."};
}
check_dimension_label_schema(label->name(), *label->schema());
}
}

// Success
return Status::Ok();
}

void ArraySchema::check_dimension_label_schema(
Expand Down Expand Up @@ -1387,17 +1378,6 @@ ArraySchema ArraySchema::deserialize(
coords_filters);
}

Status ArraySchema::init() {
// Perform check of all members
RETURN_NOT_OK(check());

// Initialize domain
RETURN_NOT_OK(domain_->init(cell_order_, tile_order_));

// Success
return Status::Ok();
}

Status ArraySchema::set_allows_dups(bool allows_dups) {
if (allows_dups && array_type_ == ArrayType::DENSE)
return LOG_STATUS(Status_ArraySchemaError(
Expand Down Expand Up @@ -1719,5 +1699,4 @@ Status ArraySchema::generate_uri(
return Status::Ok();
}

} // namespace sm
} // namespace tiledb
} // namespace tiledb::sm
18 changes: 6 additions & 12 deletions tiledb/sm/array_schema/array_schema.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,9 @@ class ArraySchema {
shared_ptr<const Attribute> shared_attribute(const std::string& name) const;

/** Returns the number of attributes. */
attribute_size_type attribute_num() const;
inline attribute_size_type attribute_num() const {
return static_cast<attribute_size_type>(attributes_.size());
}

/** Returns the attributes. */
const std::vector<shared_ptr<const Attribute>>& attributes() const;
Expand All @@ -262,9 +264,9 @@ class ArraySchema {
/**
* Checks the correctness of the array schema.
*
* @return Status
* Throws if validation fails
*/
Status check() const;
void check() const;

/**
* Throws an error if the provided schema does not match the definition given
Expand Down Expand Up @@ -504,14 +506,6 @@ class ArraySchema {
return domain_;
};

/**
* Initializes the ArraySchema object. It also performs a check to see if
* all the member attributes have been properly set.
*
* @return Status
*/
Status init();

/**
* Sets whether the array allows coordinate duplicates.
* It errors out if set to `1` for dense arrays.
Expand Down Expand Up @@ -673,7 +667,7 @@ class ArraySchema {
*/
struct attribute_reference {
const Attribute* pointer;
unsigned int index;
attribute_size_type index;
};

/**
Expand Down
4 changes: 2 additions & 2 deletions tiledb/sm/array_schema/dimension_label.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
* The MIT License
*
* @copyright Copyright (c) 2022 TileDB, Inc.
* @copyright Copyright (c) 2022-2023 TileDB, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -182,7 +182,7 @@ DimensionLabel::DimensionLabel(
throw_if_not_ok(schema_->add_attribute(label_attr));

// Check the array schema is valid.
throw_if_not_ok(schema_->check());
schema_->check();
}

// FORMAT:
Expand Down
24 changes: 1 addition & 23 deletions tiledb/sm/array_schema/domain.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
* The MIT License
*
* @copyright Copyright (c) 2017-2022 TileDB, Inc.
* @copyright Copyright (c) 2017-2023 TileDB, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -533,28 +533,6 @@ Status Domain::get_dimension_index(
"Cannot get dimension index; Invalid dimension name");
}

Status Domain::init(Layout cell_order, Layout tile_order) {
// Set cell and tile order
cell_order_ = cell_order;
tile_order_ = tile_order;

// Compute number of cells per tile
compute_cell_num_per_tile();

// Compute the tile/cell order cmp functions
set_tile_cell_order_cmp_funcs();

// Set tile_extent to empty if cell order is HILBERT
if (cell_order_ == Layout::HILBERT) {
ByteVecValue be;
for (auto& d : dimensions_) {
RETURN_NOT_OK(d->set_tile_extent(be));
}
}

return Status::Ok();
}

bool Domain::null_tile_extents() const {
for (unsigned d = 0; d < dim_num_; ++d) {
if (!tile_extent(d))
Expand Down
11 changes: 1 addition & 10 deletions tiledb/sm/array_schema/domain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
* The MIT License
*
* @copyright Copyright (c) 2017-2022 TileDB, Inc.
* @copyright Copyright (c) 2017-2023 TileDB, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -404,15 +404,6 @@ class Domain {
*/
Status get_dimension_index(const std::string& name, unsigned* dim_idx) const;

/**
* Initializes the domain.
*
* @param cell_order The cell order of the array the domain belongs to.
* @param tile_order The cell order of the array the domain belongs to.
* @return Status
*/
Status init(Layout cell_order, Layout tile_order);

/** Returns true if at least one dimension has null tile extent. */
bool null_tile_extents() const;

Expand Down
Loading

0 comments on commit 713e733

Please sign in to comment.