diff --git a/test/src/cpp-integration-query-condition.cc b/test/src/cpp-integration-query-condition.cc index a9dd0034c6ab..7768174e6680 100644 --- a/test/src/cpp-integration-query-condition.cc +++ b/test/src/cpp-integration-query-condition.cc @@ -2634,7 +2634,7 @@ TEST_CASE( CHECK(table["labs"].second == 10); for (size_t i = 0; i < table["labs"].second; ++i) { - CHECK(rlabs[i] == !!wlabs[i]); + CHECK(rlabs[i] == static_cast(wlabs[i])); } if (vfs.is_dir(array_name)) { diff --git a/test/src/unit-cppapi-consolidation-with-timestamps.cc b/test/src/unit-cppapi-consolidation-with-timestamps.cc index cbacc3b32446..7285e8bf99be 100644 --- a/test/src/unit-cppapi-consolidation-with-timestamps.cc +++ b/test/src/unit-cppapi-consolidation-with-timestamps.cc @@ -987,9 +987,11 @@ TEST_CASE_METHOD( std::vector c_a = {0, 1, 8, 2, 9, 10, 11}; std::vector c_dim1 = {1, 1, 2, 1, 2, 3, 3}; std::vector c_dim2 = {1, 2, 2, 4, 3, 2, 3}; - CHECK(!memcmp(c_a.data(), a.data(), sizeof(c_a))); - CHECK(!memcmp(c_dim1.data(), dim1.data(), sizeof(c_dim1))); - CHECK(!memcmp(c_dim2.data(), dim2.data(), sizeof(c_dim2))); + CHECK(!memcmp(c_a.data(), a.data(), c_a.size() * sizeof(int))); + CHECK( + !memcmp(c_dim1.data(), dim1.data(), c_dim1.size() * sizeof(uint64_t))); + CHECK( + !memcmp(c_dim2.data(), dim2.data(), c_dim2.size() * sizeof(uint64_t))); if (timestamps_ptr != nullptr) { std::vector exp_ts = {1, 1, 3, 1, 3, 3, 3}; CHECK(!memcmp( diff --git a/test/src/unit-filter-pipeline.cc b/test/src/unit-filter-pipeline.cc index 55eb1366c43a..04ba7d7dc412 100644 --- a/test/src/unit-filter-pipeline.cc +++ b/test/src/unit-filter-pipeline.cc @@ -3512,7 +3512,8 @@ TEST_CASE("Filter: Test XOR", "[filter][xor]") { testing_xor_filter(Datatype::INT32); testing_xor_filter(Datatype::UINT32); testing_xor_filter(Datatype::INT64); - testing_xor_filter(Datatype::UINT64); + testing_xor_filter>( + Datatype::UINT64); testing_xor_filter(Datatype::FLOAT32); testing_xor_filter(Datatype::FLOAT64); testing_xor_filter(Datatype::CHAR); diff --git a/test/src/unit-tile-metadata.cc b/test/src/unit-tile-metadata.cc index 8c37371d3299..440c92cb84ea 100644 --- a/test/src/unit-tile-metadata.cc +++ b/test/src/unit-tile-metadata.cc @@ -787,7 +787,8 @@ struct CPPVarTileMetadataFx { // Copy the string. a_offsets[i] = offset; auto idx = values[i]; - memcpy(&a_var[offset], strings_[idx].c_str(), strings_[idx].size()); + memcpy( + a_var.data() + offset, strings_[idx].c_str(), strings_[idx].size()); offset += strings_[idx].size(); // Set the coordinate value. diff --git a/tiledb/sm/fragment/fragment_metadata.cc b/tiledb/sm/fragment/fragment_metadata.cc index 4c183d680f98..0b3c9179941b 100644 --- a/tiledb/sm/fragment/fragment_metadata.cc +++ b/tiledb/sm/fragment/fragment_metadata.cc @@ -1720,6 +1720,10 @@ std::string_view FragmentMetadata::get_tile_min_as( static_cast( tile_min_var_buffer_[idx].size() - min_offset) : static_cast(offsets[tile_idx + 1] - min_offset); + if (size == 0) { + return {}; + } + char* min = &tile_min_var_buffer_[idx][min_offset]; return {min, size}; } else { @@ -1796,6 +1800,10 @@ std::string_view FragmentMetadata::get_tile_max_as( static_cast( tile_max_var_buffer_[idx].size() - max_offset) : static_cast(offsets[tile_idx + 1] - max_offset); + if (size == 0) { + return {}; + } + char* max = &tile_max_var_buffer_[idx][max_offset]; return {max, size}; } else { diff --git a/tiledb/sm/misc/comparators.h b/tiledb/sm/misc/comparators.h index e4312a360664..3bcac5efc1b2 100644 --- a/tiledb/sm/misc/comparators.h +++ b/tiledb/sm/misc/comparators.h @@ -54,10 +54,13 @@ class CellCmpBase { const Domain& domain_; /** The number of dimensions. */ - unsigned dim_num_; + const unsigned dim_num_; /** Use timestamps or not during comparison. */ - bool use_timestamps_; + const bool use_timestamps_; + + /** Enforce strict ordering for the comparator if used in a queue. */ + const bool strict_ordering_; /** Pointer to access fragment metadata. */ const std::vector>* frag_md_; @@ -66,10 +69,12 @@ class CellCmpBase { explicit CellCmpBase( const Domain& domain, const bool use_timestamps = false, + const bool strict_ordering = false, const std::vector>* frag_md = nullptr) : domain_(domain) , dim_num_(domain.dim_num()) , use_timestamps_(use_timestamps) + , strict_ordering_(strict_ordering) , frag_md_(frag_md) { } @@ -91,10 +96,6 @@ class CellCmpBase { return (*frag_md_)[f]->timestamp_range().first; } } - - inline void use_timestamps(bool enable) { - use_timestamps_ = enable; - } }; /** Wrapper of comparison function for sorting coords on row-major order. */ @@ -167,8 +168,9 @@ class HilbertCmp : public CellCmpBase { HilbertCmp( const Domain& domain, const bool use_timestamps = false, + const bool strict_ordering = false, const std::vector>* frag_md = nullptr) - : CellCmpBase(domain, use_timestamps, frag_md) { + : CellCmpBase(domain, use_timestamps, strict_ordering, frag_md) { } /** @@ -184,25 +186,38 @@ class HilbertCmp : public CellCmpBase { const GlobalOrderResultCoords& b) const { auto hilbert_a = a.tile_->hilbert_value(a.pos_); auto hilbert_b = b.tile_->hilbert_value(b.pos_); - if (hilbert_a < hilbert_b) + if (hilbert_a < hilbert_b) { return true; - else if (hilbert_a > hilbert_b) + } else if (hilbert_a > hilbert_b) { return false; + } // else the hilbert values are equal // Compare cell order on row-major to break the tie for (unsigned d = 0; d < dim_num_; ++d) { auto res = cell_order_cmp_RC(d, a, b); - if (res == -1) + if (res == -1) { return true; - if (res == 1) + } + + if (res == 1) { return false; + } // else same tile on dimension d --> continue } - // Compare timestamps - if (use_timestamps_ && get_timestamp(a) > get_timestamp(b)) { - return true; + if (use_timestamps_) { + return get_timestamp(a) > get_timestamp(b); + } else if (strict_ordering_) { + if (a.tile_->frag_idx() == b.tile_->frag_idx()) { + if (a.tile_->tile_idx() == b.tile_->tile_idx()) { + return a.pos_ > b.pos_; + } + + return a.tile_->tile_idx() > b.tile_->tile_idx(); + } + + return a.tile_->frag_idx() > b.tile_->frag_idx(); } return false; @@ -220,13 +235,16 @@ class HilbertCmpReverse { * * @param domain The array domain. * @param use_timestamps Use timestamps or not for this comparator. + * @param strict_ordering Enforce strict ordering for the comparator if used + * in a queue. * @param frag_md Pointer to the fragment metadata. */ HilbertCmpReverse( const Domain& domain, const bool use_timestamps = false, + const bool strict_ordering = false, const std::vector>* frag_md = nullptr) - : cmp_(domain, use_timestamps, frag_md) { + : cmp_(domain, use_timestamps, strict_ordering, frag_md) { } /** @@ -241,10 +259,6 @@ class HilbertCmpReverse { return !cmp_.operator()(a, b); } - inline void use_timestamps(bool enable) { - cmp_.use_timestamps(enable); - } - private: /** HilbertCmp. */ HilbertCmp cmp_; @@ -312,13 +326,17 @@ class GlobalCmp : public CellCmpBase { * Constructor. * * @param domain The array domain. + * @param use_timestamps Use timestamps or not for this comparator. + * @param strict_ordering Enforce strict ordering for the comparator if used + * in a queue. * @param frag_md Pointer to the fragment metadata. */ explicit GlobalCmp( const Domain& domain, const bool use_timestamps = false, + const bool strict_ordering = false, const std::vector>* frag_md = nullptr) - : CellCmpBase(domain, use_timestamps, frag_md) { + : CellCmpBase(domain, use_timestamps, strict_ordering, frag_md) { tile_order_ = domain.tile_order(); cell_order_ = domain.cell_order(); } @@ -388,8 +406,18 @@ class GlobalCmp : public CellCmpBase { } // Compare timestamps - if (use_timestamps_ && get_timestamp(a) > get_timestamp(b)) { - return true; + if (use_timestamps_) { + return get_timestamp(a) > get_timestamp(b); + } else if (strict_ordering_) { + if (a.tile_->frag_idx() == b.tile_->frag_idx()) { + if (a.tile_->tile_idx() == b.tile_->tile_idx()) { + return a.pos_ > b.pos_; + } + + return a.tile_->tile_idx() > b.tile_->tile_idx(); + } + + return a.tile_->frag_idx() > b.tile_->frag_idx(); } return false; @@ -413,13 +441,16 @@ class GlobalCmpReverse { * * @param domain The array domain. * @param use_timestamps Use timestamps or not for this comparator. + * @param strict_ordering Enforce strict ordering for the comparator if used + * in a queue. * @param frag_md Pointer to the fragment metadata. */ explicit GlobalCmpReverse( const Domain& domain, const bool use_timestamps = false, + const bool strict_ordering = false, const std::vector>* frag_md = nullptr) - : cmp_(domain, use_timestamps, frag_md) { + : cmp_(domain, use_timestamps, strict_ordering, frag_md) { } /** @@ -434,10 +465,6 @@ class GlobalCmpReverse { return !cmp_.operator()(a, b); } - inline void use_timestamps(bool enable) { - cmp_.use_timestamps(enable); - } - private: /** GlobalCmp. */ GlobalCmp cmp_; diff --git a/tiledb/sm/query/readers/sparse_global_order_reader.cc b/tiledb/sm/query/readers/sparse_global_order_reader.cc index c34e740973c6..28adbadecfde 100644 --- a/tiledb/sm/query/readers/sparse_global_order_reader.cc +++ b/tiledb/sm/query/readers/sparse_global_order_reader.cc @@ -737,7 +737,6 @@ bool SparseGlobalOrderReader::add_all_dups_to_queue( auto next_tile = result_tiles_it[frag_idx]; next_tile++; if (next_tile != result_tiles[frag_idx].end()) { - tile_queue.emplace(rc.tile_, rc.pos_, false); GlobalOrderResultCoords rc2(&*next_tile, 0); // All tiles should at least have one cell available. @@ -747,6 +746,8 @@ bool SparseGlobalOrderReader::add_all_dups_to_queue( // Next tile starts with the same coords, switch to it. if (rc.same_coords(rc2)) { + tile_queue.emplace(rc.tile_, rc.pos_, false); + // Remove the current tile if not used. if (!rc.tile_->used()) { tmp_read_state_.add_ignored_tile(*result_tiles_it[frag_idx]); @@ -915,7 +916,7 @@ SparseGlobalOrderReader::merge_result_cell_slabs( std::vector result_cell_slabs; CompType cmp_max_slab_length( - array_schema_.domain(), false, &fragment_metadata_); + array_schema_.domain(), false, false, &fragment_metadata_); // TODO Parallelize. @@ -929,6 +930,7 @@ SparseGlobalOrderReader::merge_result_cell_slabs( CompType cmp( array_schema_.domain(), !array_schema_.allows_dups(), + true, &fragment_metadata_); TileMinHeap tile_queue(cmp, std::move(container));