Skip to content

Commit

Permalink
Remove more duplication from buffer validation.
Browse files Browse the repository at this point in the history
  • Loading branch information
@KiterLuc
KiterLuc committed Aug 30, 2023
1 parent a59d2b5 commit 447a97f
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 80 deletions.
83 changes: 26 additions & 57 deletions tiledb/sm/query/readers/aggregators/output_buffer_validator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,29 @@ class OutputBufferValidatorStatusException : public StatusException {
}
};

void OutputBufferValidator::validate_output_buffer_arithmetic(
QueryBuffer& buffer) {
void OutputBufferValidator::validate_has_fixed_buffer(QueryBuffer& buffer) {
if (buffer.buffer_ == nullptr) {
throw OutputBufferValidatorStatusException(
"Aggregate must have a fixed size buffer.");
}
}

void OutputBufferValidator::validate_no_var_buffer(QueryBuffer& buffer) {
if (buffer.buffer_var_ != nullptr) {
throw OutputBufferValidatorStatusException(
"Aggregate must not have a var buffer.");
}
}

if (buffer.original_buffer_size_ != 8) {
void OutputBufferValidator::validate_one_element(
QueryBuffer& buffer, uint64_t element_size) {
if (buffer.original_buffer_size_ != element_size) {
throw OutputBufferValidatorStatusException(
"Aggregate fixed size buffer should be for one element only.");
}
}

void OutputBufferValidator::validate_validity(QueryBuffer& buffer) {
bool exists_validity = buffer.validity_vector_.buffer();
if (field_info_.is_nullable_) {
if (!exists_validity) {
Expand All @@ -82,21 +88,18 @@ void OutputBufferValidator::validate_output_buffer_arithmetic(
}
}

void OutputBufferValidator::validate_output_buffer_count(QueryBuffer& buffer) {
if (buffer.buffer_ == nullptr) {
throw OutputBufferValidatorStatusException(
"Count aggregates must have a fixed size buffer.");
}

if (buffer.buffer_var_ != nullptr) {
throw OutputBufferValidatorStatusException(
"Count aggregates must not have a var buffer.");
}
void OutputBufferValidator::validate_output_buffer_arithmetic(
QueryBuffer& buffer) {
validate_has_fixed_buffer(buffer);
validate_no_var_buffer(buffer);
validate_one_element(buffer, 8);
validate_validity(buffer);
}

if (buffer.original_buffer_size_ != 8) {
throw OutputBufferValidatorStatusException(
"Count aggregates fixed size buffer should be for one element only.");
}
void OutputBufferValidator::validate_output_buffer_count(QueryBuffer& buffer) {
validate_has_fixed_buffer(buffer);
validate_no_var_buffer(buffer);
validate_one_element(buffer, 8);

bool exists_validity = buffer.validity_vector_.buffer();
if (exists_validity) {
Expand All @@ -107,67 +110,33 @@ void OutputBufferValidator::validate_output_buffer_count(QueryBuffer& buffer) {

template <class T>
void OutputBufferValidator::validate_output_buffer_var(QueryBuffer& buffer) {
if (buffer.buffer_ == nullptr) {
throw OutputBufferValidatorStatusException(
"Aggregates must have a fixed size buffer.");
}
validate_has_fixed_buffer(buffer);

if (field_info_.var_sized_) {
if (buffer.buffer_var_ == nullptr) {
throw OutputBufferValidatorStatusException(
"Var sized aggregates must have a var buffer.");
}

if (buffer.original_buffer_size_ != constants::cell_var_offset_size) {
throw OutputBufferValidatorStatusException(
"Var sized aggregates offset buffer should be for one element only.");
}
validate_one_element(buffer, constants::cell_var_offset_size);

if (field_info_.cell_val_num_ != constants::var_num) {
throw OutputBufferValidatorStatusException(
"Var sized aggregates should have TILEDB_VAR_NUM cell val num.");
}
} else {
if (buffer.buffer_var_ != nullptr) {
throw OutputBufferValidatorStatusException(
"Fixed aggregates must not have a var buffer.");
}
validate_no_var_buffer(buffer);

// If cell val num is one, this is a normal fixed size attritube. If not, it
// is a fixed sized string.
if (field_info_.cell_val_num_ == 1) {
if (buffer.original_buffer_size_ != sizeof(T)) {
throw OutputBufferValidatorStatusException(
"Fixed size aggregates fixed buffer should be for one element "
"only.");
}
validate_one_element(buffer, sizeof(T));
} else {
if (buffer.original_buffer_size_ != field_info_.cell_val_num_) {
throw OutputBufferValidatorStatusException(
"Fixed size aggregates fixed buffer should be for one element "
"only.");
}
validate_one_element(buffer, field_info_.cell_val_num_);
}
}

bool exists_validity = buffer.validity_vector_.buffer();
if (field_info_.is_nullable_) {
if (!exists_validity) {
throw OutputBufferValidatorStatusException(
"Aggregates for nullable attributes must have a validity buffer.");
}

if (*buffer.validity_vector_.buffer_size() != 1) {
throw OutputBufferValidatorStatusException(
"Aggregates validity vector should be for one element only.");
}
} else {
if (exists_validity) {
throw OutputBufferValidatorStatusException(
"Aggregates for non nullable attributes must not have a validity "
"buffer.");
}
}
validate_validity(buffer);
}

// Explicit template instantiations
Expand Down
29 changes: 29 additions & 0 deletions tiledb/sm/query/readers/aggregators/output_buffer_validator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,35 @@ class OutputBufferValidator {
/* API */
/* ********************************* */

/**
* Validate the output buffer has a fixed buffer.
*
* @param buffer Output buffer.
*/
void validate_has_fixed_buffer(QueryBuffer& buffer);

/**
* Validate the output buffer has no var buffer.
*
* @param buffer Output buffer.
*/
void validate_no_var_buffer(QueryBuffer& buffer);

/**
* Validate the output buffer has one element.
*
* @param buffer Output buffer.
* @param element_size Element size.
*/
void validate_one_element(QueryBuffer& buffer, uint64_t element_size);

/**
* Validate the output buffer has the correct validity buffer.
*
* @param buffer Output buffer.
*/
void validate_validity(QueryBuffer& buffer);

/**
* Validate the output buffer can receive an arithmetic operation result.
*
Expand Down
9 changes: 4 additions & 5 deletions tiledb/sm/query/readers/aggregators/test/unit_count.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ TEST_CASE(
buffers["Count"].buffer_ = nullptr;
CHECK_THROWS_WITH(
aggregator.validate_output_buffer("Count", buffers),
"OutputBufferValidator: Count aggregates must have a fixed size "
"buffer.");
"OutputBufferValidator: Aggregate must have a fixed size buffer.");
}

SECTION("Wrong size") {
Expand All @@ -83,8 +82,8 @@ TEST_CASE(
buffers["Count"].original_buffer_size_ = 1;
CHECK_THROWS_WITH(
aggregator.validate_output_buffer("Count", buffers),
"OutputBufferValidator: Count aggregates fixed size buffer should be "
"for one element only.");
"OutputBufferValidator: Aggregate fixed size buffer should be for one "
"element only.");
}

SECTION("With var buffer") {
Expand All @@ -95,7 +94,7 @@ TEST_CASE(

CHECK_THROWS_WITH(
aggregator.validate_output_buffer("Count", buffers),
"OutputBufferValidator: Count aggregates must not have a var buffer.");
"OutputBufferValidator: Aggregate must not have a var buffer.");
}

SECTION("With validity") {
Expand Down
26 changes: 13 additions & 13 deletions tiledb/sm/query/readers/aggregators/test/unit_min_max.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ TEST_CASE(
buffers["Max"].buffer_ = nullptr;
CHECK_THROWS_WITH(
aggregator.validate_output_buffer("Max", buffers),
"OutputBufferValidator: Aggregates must have a fixed size buffer.");
"OutputBufferValidator: Aggregate must have a fixed size buffer.");
}

SECTION("Var, wrong size") {
Expand All @@ -113,8 +113,8 @@ TEST_CASE(
buffers["Max"].original_buffer_var_size_ = 8;
CHECK_THROWS_WITH(
aggregator_var.validate_output_buffer("Max", buffers),
"OutputBufferValidator: Var sized aggregates offset buffer should be "
"for one element only.");
"OutputBufferValidator: Aggregate fixed size buffer should be for one "
"element only.");
}

SECTION("Var, no var buffer") {
Expand Down Expand Up @@ -149,8 +149,8 @@ TEST_CASE(
buffers["Max"].original_buffer_size_ = 8;
CHECK_THROWS_WITH(
aggregator.validate_output_buffer("Max", buffers),
"OutputBufferValidator: Fixed size aggregates fixed buffer should be "
"for one element only.");
"OutputBufferValidator: Aggregate fixed size buffer should be for one "
"element only.");
}

SECTION("Fixed, var buffer") {
Expand All @@ -161,7 +161,7 @@ TEST_CASE(
buffers["Max"].original_buffer_var_size_ = 1;
CHECK_THROWS_WITH(
aggregator.validate_output_buffer("Max", buffers),
"OutputBufferValidator: Fixed aggregates must not have a var buffer.");
"OutputBufferValidator: Aggregate must not have a var buffer.");
}

SECTION("Fixed string, wrong size") {
Expand All @@ -170,8 +170,8 @@ TEST_CASE(
buffers["Max"].original_buffer_size_ = 4;
CHECK_THROWS_WITH(
aggregator_fixed_string.validate_output_buffer("Max", buffers),
"OutputBufferValidator: Fixed size aggregates fixed buffer should be "
"for one element only.");
"OutputBufferValidator: Aggregate fixed size buffer should be for one "
"element only.");
}

SECTION("With validity") {
Expand All @@ -184,8 +184,8 @@ TEST_CASE(
buffers["Max"].validity_vector_ = ValidityVector(&validity, &validity_size);
CHECK_THROWS_WITH(
aggregator.validate_output_buffer("Max", buffers),
"OutputBufferValidator: Aggregates for non nullable attributes must "
"not have a validity buffer.");
"OutputBufferValidator: Aggregate for non nullable attributes must not "
"have a validity buffer.");
}

SECTION("With no validity") {
Expand All @@ -195,7 +195,7 @@ TEST_CASE(

CHECK_THROWS_WITH(
aggregator_nullable.validate_output_buffer("Max", buffers),
"OutputBufferValidator: Aggregates for nullable attributes must have a "
"OutputBufferValidator: Aggregate for nullable attributes must have a "
"validity buffer.");
}

Expand All @@ -209,8 +209,8 @@ TEST_CASE(
buffers["Max"].validity_vector_ = ValidityVector(&validity, &validity_size);
CHECK_THROWS_WITH(
aggregator_nullable.validate_output_buffer("Max", buffers),
"OutputBufferValidator: Aggregates validity vector should be "
"for one element only.");
"OutputBufferValidator: Aggregate validity vector should be for one "
"element only.");
}

SECTION("Success") {
Expand Down
8 changes: 3 additions & 5 deletions tiledb/sm/query/readers/aggregators/test/unit_null_count.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ TEST_CASE(
buffers["NullCount"].buffer_ = nullptr;
CHECK_THROWS_WITH(
aggregator.validate_output_buffer("NullCount", buffers),
"OutputBufferValidator: Count aggregates must have a fixed size "
"buffer.");
"OutputBufferValidator: Aggregate must have a fixed size buffer.");
}

SECTION("Wrong size") {
Expand All @@ -97,8 +96,7 @@ TEST_CASE(
buffers["NullCount"].original_buffer_size_ = 1;
CHECK_THROWS_WITH(
aggregator.validate_output_buffer("NullCount", buffers),
"OutputBufferValidator: Count aggregates fixed size buffer should be "
"for one "
"OutputBufferValidator: Aggregate fixed size buffer should be for one "
"element only.");
}

Expand All @@ -110,7 +108,7 @@ TEST_CASE(

CHECK_THROWS_WITH(
aggregator.validate_output_buffer("NullCount", buffers),
"OutputBufferValidator: Count aggregates must not have a var buffer.");
"OutputBufferValidator: Aggregate must not have a var buffer.");
}

SECTION("With validity") {
Expand Down

0 comments on commit 447a97f

Please sign in to comment.