From f2728bad827911cde464b4b5f70bbbc2d1b70d44 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 30 Jan 2024 00:16:36 +0200 Subject: [PATCH] Clean-up `FragmentMetadata`: remove unused methods, add missing const qualifiers and return small values by value. --- tiledb/sm/fragment/fragment_metadata.cc | 43 +--- tiledb/sm/fragment/fragment_metadata.h | 202 +----------------- tiledb/sm/query/readers/dense_reader.h | 2 +- tiledb/sm/query/readers/reader_base.cc | 2 +- .../query/readers/sparse_index_reader_base.cc | 2 +- tiledb/sm/serialization/array.cc | 2 +- 6 files changed, 20 insertions(+), 233 deletions(-) diff --git a/tiledb/sm/fragment/fragment_metadata.cc b/tiledb/sm/fragment/fragment_metadata.cc index 1c92657ce8b..f191c0bcfea 100644 --- a/tiledb/sm/fragment/fragment_metadata.cc +++ b/tiledb/sm/fragment/fragment_metadata.cc @@ -1421,10 +1421,6 @@ URI FragmentMetadata::validity_uri(const std::string& name) const { encoded_name + "_validity" + constants::file_suffix); } -const std::string& FragmentMetadata::array_schema_name() { - return array_schema_name_; -} - void FragmentMetadata::load_tile_offsets( const EncryptionKey& encryption_key, std::vector& names) { // Sort 'names' in ascending order of their index. The @@ -1704,7 +1700,7 @@ uint64_t FragmentMetadata::tile_size( } uint64_t FragmentMetadata::tile_var_size( - const std::string& name, uint64_t tile_idx) { + const std::string& name, uint64_t tile_idx) const { auto it = idx_map_.find(name); assert(it != idx_map_.end()); auto idx = it->second; @@ -1924,7 +1920,8 @@ uint64_t FragmentMetadata::get_tile_null_count( return tile_null_counts_[idx][tile_idx]; } -std::vector& FragmentMetadata::get_min(const std::string& name) { +const std::vector& FragmentMetadata::get_min( + const std::string& name) const { auto it = idx_map_.find(name); assert(it != idx_map_.end()); auto idx = it->second; @@ -1946,7 +1943,8 @@ std::vector& FragmentMetadata::get_min(const std::string& name) { return fragment_mins_[idx]; } -std::vector& FragmentMetadata::get_max(const std::string& name) { +const std::vector& FragmentMetadata::get_max( + const std::string& name) const { auto it = idx_map_.find(name); assert(it != idx_map_.end()); auto idx = it->second; @@ -1968,7 +1966,7 @@ std::vector& FragmentMetadata::get_max(const std::string& name) { return fragment_maxs_[idx]; } -void* FragmentMetadata::get_sum(const std::string& name) { +const void* FragmentMetadata::get_sum(const std::string& name) const { auto it = idx_map_.find(name); assert(it != idx_map_.end()); auto idx = it->second; @@ -4696,23 +4694,6 @@ void FragmentMetadata::store_footer(const EncryptionKey&) { resources_->stats().add_counter("write_frag_meta_footer_size", tile.size()); } -void FragmentMetadata::resize_tile_offsets_vectors(uint64_t size) { - tile_offsets_mtx().resize(size); - tile_offsets().resize(size); -} - -void FragmentMetadata::resize_tile_var_offsets_vectors(uint64_t size) { - tile_var_offsets_mtx().resize(size); - tile_var_offsets().resize(size); -} - -void FragmentMetadata::resize_tile_var_sizes_vectors(uint64_t size) { - tile_var_sizes().resize(size); -} -void FragmentMetadata::resize_tile_validity_offsets_vectors(uint64_t size) { - tile_validity_offsets().resize(size); -} - void FragmentMetadata::clean_up() { auto fragment_metadata_uri = fragment_uri_.join_path(constants::fragment_metadata_filename); @@ -4721,10 +4702,6 @@ void FragmentMetadata::clean_up() { throw_if_not_ok(resources_->vfs().remove_file(fragment_metadata_uri)); } -const shared_ptr& FragmentMetadata::array_schema() const { - return array_schema_; -} - void FragmentMetadata::build_idx_map() { idx_map_.clear(); @@ -4749,14 +4726,6 @@ void FragmentMetadata::build_idx_map() { } } -void FragmentMetadata::set_schema_name(const std::string& name) { - array_schema_name_ = name; -} - -void FragmentMetadata::set_dense(bool dense) { - dense_ = dense; -} - // Explicit template instantiations template std::vector> FragmentMetadata::compute_overlapping_tile_ids_cov( diff --git a/tiledb/sm/fragment/fragment_metadata.h b/tiledb/sm/fragment/fragment_metadata.h index 46bc5a72729..2ab0821807f 100644 --- a/tiledb/sm/fragment/fragment_metadata.h +++ b/tiledb/sm/fragment/fragment_metadata.h @@ -342,7 +342,7 @@ class FragmentMetadata { return has_delete_meta_; } - inline bool has_tile_metadata() { + inline bool has_tile_metadata() const { return version_ >= constants::tile_metadata_min_version; } @@ -762,12 +762,6 @@ class FragmentMetadata { */ void set_array_schema(const shared_ptr& array_schema); - /** Sets the array_schema name */ - void set_schema_name(const std::string& name); - - /** Sets the internal dense_ field*/ - void set_dense(bool dense); - /** Returns the number of tiles in the fragment. */ uint64_t tile_num() const; @@ -781,7 +775,9 @@ class FragmentMetadata { URI validity_uri(const std::string& name) const; /** Return the array schema name. */ - const std::string& array_schema_name(); + const std::string& array_schema_name() const { + return array_schema_name_; + } /** * Retrieves the starting offset of the input tile of the input attribute @@ -878,7 +874,7 @@ class FragmentMetadata { * @param tile_idx The index of the tile in the metadata. * @return Size. */ - uint64_t tile_var_size(const std::string& name, uint64_t tile_idx); + uint64_t tile_var_size(const std::string& name, uint64_t tile_idx) const; /** * Retrieves the tile min value for a given attribute or dimension and tile @@ -930,7 +926,7 @@ class FragmentMetadata { * @param name The input attribute/dimension. * @return Value. */ - std::vector& get_min(const std::string& name); + const std::vector& get_min(const std::string& name) const; /** * Retrieves the max value for a given attribute or dimension. @@ -938,7 +934,7 @@ class FragmentMetadata { * @param name The input attribute/dimension. * @return Value. */ - std::vector& get_max(const std::string& name); + const std::vector& get_max(const std::string& name) const; /** * Retrieves the sum value for a given attribute or dimension. @@ -946,7 +942,7 @@ class FragmentMetadata { * @param name The input attribute/dimension. * @return Sum. */ - void* get_sum(const std::string& name); + const void* get_sum(const std::string& name) const; /** * Retrieves the null count value for a given attribute or dimension. @@ -1110,161 +1106,8 @@ class FragmentMetadata { * * @return */ - const shared_ptr& array_schema() const; - - /** File sizes accessor */ - std::vector& file_sizes() { - return file_sizes_; - } - - /** File var sizes accessor */ - std::vector& file_var_sizes() { - return file_var_sizes_; - } - - /** File validity sizes accessor */ - std::vector& file_validity_sizes() { - return file_validity_sizes_; - } - - /** Fragment uri accessor */ - URI& fragment_uri() { - return fragment_uri_; - } - - /** has_timestamps accessor */ - bool& has_timestamps() { - return has_timestamps_; - } - - /** has_delete_meta accessor */ - bool& has_delete_meta() { - return has_delete_meta_; - } - - /** has_consolidated_footer accessor */ - bool& has_consolidated_footer() { - return has_consolidated_footer_; - } - - /** sparse_tile_num accessor */ - uint64_t& sparse_tile_num() { - return sparse_tile_num_; - } - - /** tile_index_base accessor */ - uint64_t& tile_index_base() { - return tile_index_base_; - } - - /** tile_offsets accessor */ - std::vector>& tile_offsets() { - return tile_offsets_; - } - - /** tile_offsets_mtx accessor */ - std::deque& tile_offsets_mtx() { - return tile_offsets_mtx_; - } - - /** tile_var_offsets accessor */ - std::vector>& tile_var_offsets() { - return tile_var_offsets_; - } - - /** tile_var_offsets_mtx accessor */ - std::deque& tile_var_offsets_mtx() { - return tile_var_offsets_mtx_; - } - - /** tile_var_sizes accessor */ - std::vector>& tile_var_sizes() { - return tile_var_sizes_; - } - - /** tile_validity_offsets accessor */ - std::vector>& tile_validity_offsets() { - return tile_validity_offsets_; - } - - /** tile_min_buffer accessor */ - std::vector>& tile_min_buffer() { - return tile_min_buffer_; - } - - /** tile_min_var_buffer accessor */ - std::vector>& tile_min_var_buffer() { - return tile_min_var_buffer_; - } - - /** tile_max_buffer accessor */ - std::vector>& tile_max_buffer() { - return tile_max_buffer_; - } - - /** tile_max_var_buffer accessor */ - std::vector>& tile_max_var_buffer() { - return tile_max_var_buffer_; - } - - /** tile_sums accessor */ - std::vector>& tile_sums() { - return tile_sums_; - } - - /** tile_null_counts accessor */ - std::vector>& tile_null_counts() { - return tile_null_counts_; - } - - /** fragment_mins accessor */ - std::vector>& fragment_mins() { - return fragment_mins_; - } - - /** fragment_maxs accessor */ - std::vector>& fragment_maxs() { - return fragment_maxs_; - } - - /** fragment_sums accessor */ - std::vector& fragment_sums() { - return fragment_sums_; - } - - /** fragment_null_counts accessor */ - std::vector& fragment_null_counts() { - return fragment_null_counts_; - } - - /** version accessor */ - uint32_t& version() { - return version_; - } - - /** timestamp_range accessor */ - std::pair& timestamp_range() { - return timestamp_range_; - } - - /** last_tile_cell_num accessor */ - uint64_t& last_tile_cell_num() { - return last_tile_cell_num_; - } - - /** non_empty_domain accessor */ - NDRange& non_empty_domain() { - return non_empty_domain_; - } - - /** rtree accessor */ - RTree& rtree() { - return rtree_; - } - - /** gt_offsets_ accessor */ - inline GenericTileOffsets& generic_tile_offsets() { - return gt_offsets_; + const shared_ptr& array_schema() const { + return array_schema_; } /** set the CR pointer during deserialization*/ @@ -1272,11 +1115,6 @@ class FragmentMetadata { resources_ = cr; } - /** set the memory tracker pointer during deserialization*/ - void set_memory_tracker(MemoryTracker* memory_tracker) { - memory_tracker_ = memory_tracker; - } - /** loaded_metadata_.rtree_ accessor */ void set_rtree_loaded() { loaded_metadata_.rtree_ = true; @@ -1287,26 +1125,6 @@ class FragmentMetadata { loaded_metadata_ = loaded_metadata; } - /** - * Resize tile offsets related vectors. - */ - void resize_tile_offsets_vectors(uint64_t size); - - /** - * Resize tile var offsets related vectors. - */ - void resize_tile_var_offsets_vectors(uint64_t size); - - /** - * Resize tile var sizes related vectors. - */ - void resize_tile_var_sizes_vectors(uint64_t size); - - /** - * Resize tile validity offsets related vectors. - */ - void resize_tile_validity_offsets_vectors(uint64_t size); - private: /* ********************************* */ /* PRIVATE ATTRIBUTES */ diff --git a/tiledb/sm/query/readers/dense_reader.h b/tiledb/sm/query/readers/dense_reader.h index 16c997299ac..c76f3f7a22a 100644 --- a/tiledb/sm/query/readers/dense_reader.h +++ b/tiledb/sm/query/readers/dense_reader.h @@ -309,7 +309,7 @@ class DenseReader : public ReaderBase, public IQueryStrategy { if ((type == Datatype::STRING_ASCII || type == Datatype::CHAR) && array_schema_.cell_val_num(name) != constants::var_num && array_schema_.is_nullable(name)) { - if (frag_md->version() <= 20) { + if (frag_md->format_version() <= 20) { return false; } } diff --git a/tiledb/sm/query/readers/reader_base.cc b/tiledb/sm/query/readers/reader_base.cc index 7f6eb29c5e8..4bbe568b314 100644 --- a/tiledb/sm/query/readers/reader_base.cc +++ b/tiledb/sm/query/readers/reader_base.cc @@ -227,7 +227,7 @@ bool ReaderBase::need_timestamped_conditions() { for (auto& delete_and_update_condition : delete_and_update_conditions_) { auto delete_timestamp = delete_and_update_condition.condition_timestamp(); - auto& frag_timestamps = fragment_metadata_[i]->timestamp_range(); + auto frag_timestamps = fragment_metadata_[i]->timestamp_range(); if (delete_timestamp >= frag_timestamps.first && delete_timestamp <= frag_timestamps.second) { make_timestamped_conditions = true; diff --git a/tiledb/sm/query/readers/sparse_index_reader_base.cc b/tiledb/sm/query/readers/sparse_index_reader_base.cc index b47e6450359..e958c5a844a 100644 --- a/tiledb/sm/query/readers/sparse_index_reader_base.cc +++ b/tiledb/sm/query/readers/sparse_index_reader_base.cc @@ -171,7 +171,7 @@ std::vector SparseIndexReaderBase::tile_offset_sizes() { // For fragments with version smaller than 5 we have zipped coords. // Otherwise we load each dimensions independently. - if (fragment->version() < 5) { + if (fragment->format_version() < 5) { num = 1; } else { for (unsigned d = 0; d < dim_num; ++d) { diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc index 089c00d9ce5..1bba85cd166 100644 --- a/tiledb/sm/serialization/array.cc +++ b/tiledb/sm/serialization/array.cc @@ -183,7 +183,7 @@ Status array_to_capnp( // Old fragment with zipped coordinates didn't have a format that // allow to dynamically load tile offsets and sizes and since they all // get loaded at array open, we need to serialize them here. - if (fragment_metadata_all[i]->version() <= 2) { + if (fragment_metadata_all[i]->format_version() <= 2) { fragment_meta_sizes_offsets_to_capnp( *fragment_metadata_all[i], &fragment_metadata_builder); }