Skip to content

Commit

Permalink
Fix container issues in windows builds. (TileDB-Inc#4177)
Browse files Browse the repository at this point in the history
This fixes various container issues when running the windows build/tests.

---
TYPE: BUG
DESC: Fix container issues in windows builds.
  • Loading branch information
KiterLuc authored and davisp committed Sep 6, 2023
1 parent 713e733 commit 1eebb27
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 35 deletions.
2 changes: 1 addition & 1 deletion test/src/cpp-integration-query-condition.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>(wlabs[i]));
}

if (vfs.is_dir(array_name)) {
Expand Down
8 changes: 5 additions & 3 deletions test/src/unit-cppapi-consolidation-with-timestamps.cc
Original file line number Diff line number Diff line change
Expand Up @@ -987,9 +987,11 @@ TEST_CASE_METHOD(
std::vector<int> c_a = {0, 1, 8, 2, 9, 10, 11};
std::vector<uint64_t> c_dim1 = {1, 1, 2, 1, 2, 3, 3};
std::vector<uint64_t> 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<uint64_t> exp_ts = {1, 1, 3, 1, 3, 3, 3};
CHECK(!memcmp(
Expand Down
3 changes: 2 additions & 1 deletion test/src/unit-filter-pipeline.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3512,7 +3512,8 @@ TEST_CASE("Filter: Test XOR", "[filter][xor]") {
testing_xor_filter<int32_t>(Datatype::INT32);
testing_xor_filter<uint32_t>(Datatype::UINT32);
testing_xor_filter<int64_t>(Datatype::INT64);
testing_xor_filter<uint64_t>(Datatype::UINT64);
testing_xor_filter<uint64_t, std::uniform_int_distribution<uint64_t>>(
Datatype::UINT64);
testing_xor_filter<float, FloatDistribution>(Datatype::FLOAT32);
testing_xor_filter<double, FloatDistribution>(Datatype::FLOAT64);
testing_xor_filter<char>(Datatype::CHAR);
Expand Down
3 changes: 2 additions & 1 deletion test/src/unit-tile-metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions tiledb/sm/fragment/fragment_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1720,6 +1720,10 @@ std::string_view FragmentMetadata::get_tile_min_as<std::string_view>(
static_cast<sv_size_cast>(
tile_min_var_buffer_[idx].size() - min_offset) :
static_cast<sv_size_cast>(offsets[tile_idx + 1] - min_offset);
if (size == 0) {
return {};
}

char* min = &tile_min_var_buffer_[idx][min_offset];
return {min, size};
} else {
Expand Down Expand Up @@ -1796,6 +1800,10 @@ std::string_view FragmentMetadata::get_tile_max_as<std::string_view>(
static_cast<sv_size_cast>(
tile_max_var_buffer_[idx].size() - max_offset) :
static_cast<sv_size_cast>(offsets[tile_idx + 1] - max_offset);
if (size == 0) {
return {};
}

char* max = &tile_max_var_buffer_[idx][max_offset];
return {max, size};
} else {
Expand Down
81 changes: 54 additions & 27 deletions tiledb/sm/misc/comparators.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<shared_ptr<FragmentMetadata>>* frag_md_;
Expand All @@ -66,10 +69,12 @@ class CellCmpBase {
explicit CellCmpBase(
const Domain& domain,
const bool use_timestamps = false,
const bool strict_ordering = false,
const std::vector<shared_ptr<FragmentMetadata>>* frag_md = nullptr)
: domain_(domain)
, dim_num_(domain.dim_num())
, use_timestamps_(use_timestamps)
, strict_ordering_(strict_ordering)
, frag_md_(frag_md) {
}

Expand All @@ -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. */
Expand Down Expand Up @@ -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<shared_ptr<FragmentMetadata>>* frag_md = nullptr)
: CellCmpBase(domain, use_timestamps, frag_md) {
: CellCmpBase(domain, use_timestamps, strict_ordering, frag_md) {
}

/**
Expand All @@ -184,25 +186,38 @@ class HilbertCmp : public CellCmpBase {
const GlobalOrderResultCoords<BitmapType>& 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;
Expand All @@ -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<shared_ptr<FragmentMetadata>>* frag_md = nullptr)
: cmp_(domain, use_timestamps, frag_md) {
: cmp_(domain, use_timestamps, strict_ordering, frag_md) {
}

/**
Expand All @@ -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_;
Expand Down Expand Up @@ -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<shared_ptr<FragmentMetadata>>* 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();
}
Expand Down Expand Up @@ -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;
Expand All @@ -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<shared_ptr<FragmentMetadata>>* frag_md = nullptr)
: cmp_(domain, use_timestamps, frag_md) {
: cmp_(domain, use_timestamps, strict_ordering, frag_md) {
}

/**
Expand All @@ -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_;
Expand Down
6 changes: 4 additions & 2 deletions tiledb/sm/query/readers/sparse_global_order_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,6 @@ bool SparseGlobalOrderReader<BitmapType>::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.
Expand All @@ -747,6 +746,8 @@ bool SparseGlobalOrderReader<BitmapType>::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]);
Expand Down Expand Up @@ -915,7 +916,7 @@ SparseGlobalOrderReader<BitmapType>::merge_result_cell_slabs(

std::vector<ResultCellSlab> result_cell_slabs;
CompType cmp_max_slab_length(
array_schema_.domain(), false, &fragment_metadata_);
array_schema_.domain(), false, false, &fragment_metadata_);

// TODO Parallelize.

Expand All @@ -929,6 +930,7 @@ SparseGlobalOrderReader<BitmapType>::merge_result_cell_slabs(
CompType cmp(
array_schema_.domain(),
!array_schema_.allows_dups(),
true,
&fragment_metadata_);
TileMinHeap<CompType> tile_queue(cmp, std::move(container));

Expand Down

0 comments on commit 1eebb27

Please sign in to comment.