From 8acbf07dcc557e1573d5fa2841134b67bb622dd1 Mon Sep 17 00:00:00 2001 From: Luc Rancourt Date: Fri, 1 Sep 2023 19:24:33 +0200 Subject: [PATCH] Addressing feedback from @davisp. --- tiledb/sm/query/readers/reader_base.cc | 6 +- tiledb/sm/query/test/unit_query_condition.cc | 3 - tiledb/sm/tile/tile.cc | 76 ++++++++++++-------- tiledb/sm/tile/tile.h | 34 +++++++-- 4 files changed, 80 insertions(+), 39 deletions(-) diff --git a/tiledb/sm/query/readers/reader_base.cc b/tiledb/sm/query/readers/reader_base.cc index cf5cca70e85..a04cf04c5b1 100644 --- a/tiledb/sm/query/readers/reader_base.cc +++ b/tiledb/sm/query/readers/reader_base.cc @@ -718,7 +718,11 @@ ReaderBase::load_tile_chunk_data( const FilterPipeline& filters = array_schema_.filters(name); if (!var_size || !filters.skip_offsets_filtering(t_var->type(), array_schema_.version())) { - unfiltered_tile_size = t->load_chunk_data(tile_chunk_data, var_size); + if (var_size) { + unfiltered_tile_size = t->load_offsets_chunk_data(tile_chunk_data); + } else { + unfiltered_tile_size = t->load_chunk_data(tile_chunk_data); + } } if (var_size) { diff --git a/tiledb/sm/query/test/unit_query_condition.cc b/tiledb/sm/query/test/unit_query_condition.cc index 1c4837266bb..8c5fb6260db 100644 --- a/tiledb/sm/query/test/unit_query_condition.cc +++ b/tiledb/sm/query/test/unit_query_condition.cc @@ -2675,9 +2675,6 @@ void test_apply_cells_sparse( auto expected_iter = expected_cell_idx_vec.begin(); for (uint64_t cell_idx = 0; cell_idx < cells; ++cell_idx) { if (result_bitmap[cell_idx]) { - if (*expected_iter != cell_idx) { - std::cout << "hello"; - } REQUIRE(*expected_iter == cell_idx); ++expected_iter; } diff --git a/tiledb/sm/tile/tile.cc b/tiledb/sm/tile/tile.cc index 3a15ec1b43f..b873858e300 100644 --- a/tiledb/sm/tile/tile.cc +++ b/tiledb/sm/tile/tile.cc @@ -249,38 +249,17 @@ Status Tile::zip_coordinates() { return Status::Ok(); } -uint64_t Tile::load_chunk_data(ChunkData& unfiltered_tile, bool is_offsets) { - assert(filtered()); - - Deserializer deserializer(filtered_data(), filtered_size()); - - // Make a pass over the tile to get the chunk information. - uint64_t num_chunks = deserializer.read(); - - auto& filtered_chunks = unfiltered_tile.filtered_chunks_; - auto& chunk_offsets = unfiltered_tile.chunk_offsets_; - filtered_chunks.resize(num_chunks); - chunk_offsets.resize(num_chunks); - uint64_t total_orig_size = 0; - for (uint64_t i = 0; i < num_chunks; i++) { - auto& chunk = filtered_chunks[i]; - chunk.unfiltered_data_size_ = deserializer.read(); - chunk.filtered_data_size_ = deserializer.read(); - chunk.filtered_metadata_size_ = deserializer.read(); - chunk.filtered_metadata_ = const_cast( - deserializer.get_ptr(chunk.filtered_metadata_size_)); - chunk.filtered_data_ = const_cast( - deserializer.get_ptr(chunk.filtered_data_size_)); - - chunk_offsets[i] = total_orig_size; - total_orig_size += chunk.unfiltered_data_size_; - } +uint64_t Tile::load_chunk_data(ChunkData& chunk_data) { + return load_chunk_data(chunk_data, size()); +} - if (total_orig_size + (is_offsets ? sizeof(uint64_t) : 0) != size()) { - throw TileStatusException("Incorrect unfiltered tile size allocated."); +uint64_t Tile::load_offsets_chunk_data(ChunkData& chunk_data) { + auto s = size(); + if (s < 8) { + throw std::runtime_error("Offsets tile should at least be 8 bytes."); } - return total_orig_size; + return load_chunk_data(chunk_data, s - 8); } void Tile::swap(Tile& tile) { @@ -320,5 +299,44 @@ void WriterTile::swap(WriterTile& tile) { std::swap(filtered_buffer_, tile.filtered_buffer_); } +/* ********************************* */ +/* PRIVATE FUNCTIONS */ +/* ********************************* */ + +uint64_t Tile::load_chunk_data( + ChunkData& unfiltered_tile, uint64_t expected_original_size) { + assert(filtered()); + + Deserializer deserializer(filtered_data(), filtered_size()); + + // Make a pass over the tile to get the chunk information. + uint64_t num_chunks = deserializer.read(); + + auto& filtered_chunks = unfiltered_tile.filtered_chunks_; + auto& chunk_offsets = unfiltered_tile.chunk_offsets_; + filtered_chunks.resize(num_chunks); + chunk_offsets.resize(num_chunks); + uint64_t total_orig_size = 0; + for (uint64_t i = 0; i < num_chunks; i++) { + auto& chunk = filtered_chunks[i]; + chunk.unfiltered_data_size_ = deserializer.read(); + chunk.filtered_data_size_ = deserializer.read(); + chunk.filtered_metadata_size_ = deserializer.read(); + chunk.filtered_metadata_ = const_cast( + deserializer.get_ptr(chunk.filtered_metadata_size_)); + chunk.filtered_data_ = const_cast( + deserializer.get_ptr(chunk.filtered_data_size_)); + + chunk_offsets[i] = total_orig_size; + total_orig_size += chunk.unfiltered_data_size_; + } + + if (total_orig_size != expected_original_size) { + throw TileStatusException("Incorrect unfiltered tile size allocated."); + } + + return total_orig_size; +} + } // namespace sm } // namespace tiledb diff --git a/tiledb/sm/tile/tile.h b/tiledb/sm/tile/tile.h index db2faae3e95..0360be55d0c 100644 --- a/tiledb/sm/tile/tile.h +++ b/tiledb/sm/tile/tile.h @@ -267,6 +267,31 @@ class Tile : public TileBase { */ Status zip_coordinates(); + /** + * Reads the chunk data of a tile buffer and populates a chunk data structure. + * + * @param chunk_data Tile chunk info, buffers and offsets. + * @return Original size. + */ + uint64_t load_chunk_data(ChunkData& chunk_data); + + /** + * Reads the chunk data of a tile offsets buffer and populates a chunk data + * structure. + * + * @param chunk_data Tile chunk info, buffers and offsets. + * @return Original size. + */ + uint64_t load_offsets_chunk_data(ChunkData& chunk_data); + + /** Swaps the contents (all field values) of this tile with the given tile. */ + void swap(Tile& tile); + + private: + /* ********************************* */ + /* PRIVATE FUNCTIONS */ + /* ********************************* */ + /** * Reads the chunk data of a tile buffer and populates a chunk data structure. * @@ -291,15 +316,12 @@ class Tile : public TileBase { * chunkN_data (uint8_t[]) * * @param chunk_data Tile chunk info, buffers and offsets. - * @param is_offsets Does the tile contains offsets? + * @param expected_original_size Expected size for the tile. * @return Original size. */ - uint64_t load_chunk_data(ChunkData& chunk_data, bool is_offsets = false); - - /** Swaps the contents (all field values) of this tile with the given tile. */ - void swap(Tile& tile); + uint64_t load_chunk_data( + ChunkData& chunk_data, uint64_t expected_original_size); - private: /* ********************************* */ /* PRIVATE ATTRIBUTES */ /* ********************************* */