Bugfix: Empty type

cpp/arcticdb/CMakeLists.txt

@@ -157,7 +157,7 @@ ENDIF()
 
 set(CMAKE_INSTALL_RPATH "\\$ORIGIN/lib")
 
-if (NOT WIN32)
+if (NOT MSVC)
  if(NOT ${ARCTICDB_USING_CONDA})
  # Compilers on conda-forge are relatively strict.
  # For now, we do not want to treat warnings as errors for those builds.
@@ -166,6 +166,13 @@ if (NOT WIN32)
  add_compile_options("-ggdb")
  add_compile_options("-fvisibility=hidden")
  add_compile_options("-fno-omit-frame-pointer")
+else()
+ add_compile_options(
+ /WX
+ /wd4250 # Suppress inherits via dominance warning
+ /we4189 # Unused local variable is an error
+ /we4100 # Unused function parameter is an error
+ )
 endif ()
 
 ## Core library without python bindings ##
@@ -200,6 +207,7 @@ set(arcticdb_srcs
  entity/key.hpp
  entity/metrics.hpp
  entity/data_error.hpp
+ entity/descriptors.hpp
  entity/native_tensor.hpp
  entity/performance_tracing.hpp
  entity/protobuf_mappings.hpp
@@ -208,6 +216,7 @@ set(arcticdb_srcs
  entity/serialized_key.hpp
  entity/type_conversion.hpp
  entity/types.hpp
+ entity/type_utils.hpp
  entity/types-inl.hpp
  entity/variant_key.hpp
  entity/versioned_item.hpp
@@ -233,7 +242,6 @@ set(arcticdb_srcs
  pipeline/read_pipeline.hpp
  pipeline/slicing.hpp
  pipeline/string_pool_utils.hpp
- pipeline/string_pool_utils.hpp
  pipeline/value.hpp
  pipeline/value_set.hpp
  pipeline/write_frame.hpp
@@ -376,7 +384,9 @@ set(arcticdb_srcs
  entity/metrics.cpp
  entity/performance_tracing.cpp
  entity/types.cpp
+ entity/type_utils.cpp
  log/log.cpp
+ pipeline/column_mapping.cpp
  pipeline/column_stats.cpp
  pipeline/frame_slice.cpp
  pipeline/frame_utils.cpp

cpp/arcticdb/codec/codec-inl.hpp

@@ -18,10 +18,6 @@
 
 #include <arcticdb/util/bitset.hpp>
 
-#include <arcticdb/util/buffer.hpp>
-
-#include <google/protobuf/text_format.h>
-
 #include <type_traits>
 
 namespace arcticdb {

cpp/arcticdb/codec/codec.cpp

@@ -310,11 +310,12 @@ void decode_string_pool( const arcticdb::proto::encoding::SegmentHeader& hdr,
  }
 }
 
-void decode_v2(const Segment& segment,
- arcticdb::proto::encoding::SegmentHeader& hdr,
- SegmentInMemory& res,
- const StreamDescriptor& desc)
- {
+void decode_v2(
+ const Segment& segment,
+ arcticdb::proto::encoding::SegmentHeader& hdr,
+ SegmentInMemory& res,
+ const StreamDescriptor& desc
+) {
  ARCTICDB_SAMPLE(DecodeSegment, 0)
  const auto [begin, end] = get_segment_begin_end(segment, hdr);
  auto encoded_fields_ptr = end;
@@ -364,11 +365,12 @@ void decode_v2(const Segment& segment,
  res.set_compacted(segment.header().compacted());
  }}
 
-void decode_v1(const Segment& segment,
- const arcticdb::proto::encoding::SegmentHeader& hdr,
- SegmentInMemory& res,
- StreamDescriptor::Proto& desc)
-{
+void decode_v1(
+ const Segment& segment,
+ const arcticdb::proto::encoding::SegmentHeader& hdr,
+ SegmentInMemory& res,
+ StreamDescriptor::Proto& desc
+) {
  ARCTICDB_SAMPLE(DecodeSegment, 0)
  const uint8_t* data = segment.buffer().data();
  util::check(data != nullptr, "Got null data ptr from segment");
@@ -384,17 +386,29 @@ void decode_v1(const Segment& segment,
  const auto seg_row_count = fields_size ? ssize_t(hdr.fields(0).ndarray().items_count()) : 0LL;
  res.init_column_map();
 
- for (std::size_t i = 0; i < static_cast<size_t>(fields_size); ++i) {
- const auto& field = hdr.fields(static_cast<int>(i));
- const auto& field_name = desc.fields(static_cast<int>(i)).name();
- util::check(data!=end, "Reached end of input block with {} fields to decode", fields_size-i);
- if(auto col_index = res.column_index(field_name)) {
+ for (int i = 0; i < fields_size; ++i) {
+ const auto& field = hdr.fields(i);
+ const auto& field_name = desc.fields(i).name();
+ if (auto col_index = res.column_index(field_name)) {
  auto& col = res.column(static_cast<position_t>(*col_index));
- data += decode_field(res.field(*col_index).type(), field, data, col, col.opt_sparse_map(), to_encoding_version(hdr.encoding_version()));
- } else
+ util::check(
+ data != end || is_empty_type(col.type().data_type()),
+ "Reached end of input block with {} fields to decode",
+ fields_size - i
+ );
+ data += decode_field(
+ res.field(*col_index).type(),
+ field,
+ data,
+ col,
+ col.opt_sparse_map(),
+ to_encoding_version(hdr.encoding_version())
+ );
+ } else {
+ util::check(data != end, "Reached end of input block with {} fields to decode", fields_size - i);
  data += encoding_sizes::field_compressed_size(field);
-
- ARCTICDB_TRACE(log::codec(), "Decoded column {} to position {}", i, data-begin);
+ }
+ ARCTICDB_TRACE(log::codec(), "Decoded column {} to position {}", i, data - begin);
  }
 
  decode_string_pool(hdr, data, begin, end, res);

cpp/arcticdb/codec/zstd.hpp

@@ -51,28 +51,31 @@ struct ZstdBlockEncoder {
 
 struct ZstdDecoder {
 
-/*
- * encoder_version is here to support multiple versions but won't be used before we have them
- */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wunused-parameter"
-
- template<typename T>
+ /// @param[in] encoder_version Used to support multiple versions but won't be used before we have them
+ template <typename T>
  static void decode_block(
- std::uint32_t encoder_version,
- const std::uint8_t *in,
+ [[maybe_unused]] std::uint32_t encoder_version,
+ const std::uint8_t* in,
  std::size_t in_bytes,
- T *t_out,
- std::size_t out_bytes) {
+ T* t_out,
+ std::size_t out_bytes
+ ) {
 
  const std::size_t decomp_size = ZSTD_getFrameContentSize(in, in_bytes);
- codec::check<ErrorCode::E_DECODE_ERROR>(decomp_size == out_bytes, "expected out_bytes == zstd deduced bytes, actual {} != {}",
- out_bytes, decomp_size);
+ codec::check<ErrorCode::E_DECODE_ERROR>(
+ decomp_size == out_bytes,
+ "expected out_bytes == zstd deduced bytes, actual {} != {}",
+ out_bytes,
+ decomp_size
+ );
  std::size_t real_decomp = ZSTD_decompress(t_out, out_bytes, in, in_bytes);
- codec::check<ErrorCode::E_DECODE_ERROR>(real_decomp == out_bytes, "expected out_bytes == zstd decompressed bytes, actual {} != {}",
- out_bytes, real_decomp);
+ codec::check<ErrorCode::E_DECODE_ERROR>(
+ real_decomp == out_bytes,
+ "expected out_bytes == zstd decompressed bytes, actual {} != {}",
+ out_bytes,
+ real_decomp
+ );
  }
-#pragma GCC diagnostic pop
 };
 
 } // namespace arcticdb::detail

cpp/arcticdb/column_store/column.cpp

@@ -90,14 +90,14 @@ void Column::unsparsify(size_t num_rows) {
  if(!sparse_map_)
  return;
 
- type_.visit_tag([that=this, num_rows] (const auto tdt) {
+ type_.visit_tag([this, num_rows] (const auto tdt) {
  using TagType = decltype(tdt);
  using RawType = typename TagType::DataTypeTag::raw_type;
  const auto dest_bytes = num_rows * sizeof(RawType);
  auto dest = ChunkedBuffer::presized(dest_bytes);
  util::default_initialize<TagType>(dest.data(), dest_bytes);
- util::expand_dense_buffer_using_bitmap<RawType>(that->sparse_map_.value(), that->data_.buffer().data(), dest.data());
- std::swap(dest, that->data_.buffer());
+ util::expand_dense_buffer_using_bitmap<RawType>(sparse_map_.value(), data_.buffer().data(), dest.data());
+ std::swap(dest, data_.buffer());
  });
  sparse_map_ = std::nullopt;
  last_logical_row_ = last_physical_row_ = static_cast<ssize_t>(num_rows) - 1;
@@ -106,14 +106,13 @@ void Column::unsparsify(size_t num_rows) {
 }
 
 void Column::sparsify() {
- type().visit_tag([that=this](auto type_desc_tag) {
- using TDT = decltype(type_desc_tag);
- using RawType = typename TDT::DataTypeTag::raw_type;
- if constexpr(is_floating_point_type(TDT::DataTypeTag::data_type)) {
- auto raw_ptr =reinterpret_cast<const RawType*>(that->ptr());
- auto buffer = util::scan_floating_point_to_sparse(raw_ptr, that->row_count(), that->sparse_map());
- std::swap(that->data().buffer(), buffer);
- that->last_physical_row_ = that->sparse_map().count() - 1;
+ type().visit_tag([this](auto type_desc_tag) {
+ using RawType = typename decltype(type_desc_tag)::DataTypeTag::raw_type;
+ if constexpr (is_floating_point_type(type_desc_tag.data_type())) {
+ auto raw_ptr = reinterpret_cast<const RawType*>(ptr());
+ auto buffer = util::scan_floating_point_to_sparse(raw_ptr, row_count(), sparse_map());
+ std::swap(data().buffer(), buffer);
+ last_physical_row_ = sparse_map().count() - 1;
  }
  });
 }
@@ -513,65 +512,89 @@ std::vector<std::shared_ptr<Column>> Column::split(const std::shared_ptr<Column>
  return output;
 }
 
-// Produces a new column
-// Inclusive of start_row, exclusive of end_row
-std::shared_ptr<Column> Column::truncate(const std::shared_ptr<Column>& column, size_t start_row, size_t end_row) {
- auto type_size = get_type_size(column->type().data_type());
- // Bytes from the underlying chunked buffer to include. Inclusive of start_byte, exclusive of end_byte
- size_t start_byte;
- size_t end_byte;
- util::BitMagic input_sparse_map;
- if (column->is_sparse()) {
- input_sparse_map = column->sparse_map();
- internal::check<ErrorCode::E_ASSERTION_FAILURE>(input_sparse_map.size() > 0, "Unexpected empty sparse map in Column::truncate");
- // Sparse columns do not include trailing 0s in the bitset, so the relevant end_row is capped at the size of the biset
+/// Bytes from the underlying chunked buffer to include when truncating. Inclusive of start_byte, exclusive of end_byte
+[[nodiscard]] static std::pair<size_t, size_t> column_start_end_bytes(
+ const Column& column,
+ size_t start_row,
+ size_t end_row
+) {
+ const size_t type_size = get_type_size(column.type().data_type());
+ size_t start_byte = start_row * type_size;
+ size_t end_byte = end_row * type_size;
+ if (column.is_sparse()) {
+ const util::BitMagic& input_sparse_map = column.sparse_map();
+ internal::check<ErrorCode::E_ASSERTION_FAILURE>(
+ input_sparse_map.size() > 0,
+ "Unexpected empty sparse map in Column::truncate"
+ );
+ // Sparse columns do not include trailing 0s in the bitset, so the relevant end_row is capped at the size of the
+ // biset
  end_row = std::min(end_row, static_cast<size_t>(input_sparse_map.size()));
  // count_range is inclusive at both ends
- auto set_bits_before_range = start_row > 0 ? input_sparse_map.count_range(0, start_row - 1) : 0;
- auto set_bits_in_range = input_sparse_map.count_range(start_row, end_row - 1);
+ const size_t set_bits_before_range = start_row > 0 ? input_sparse_map.count_range(0, start_row - 1) : 0;
+ const size_t set_bits_in_range = input_sparse_map.count_range(start_row, end_row - 1);
  start_byte = set_bits_before_range * type_size;
  end_byte = start_byte + (set_bits_in_range * type_size);
- } else {
- start_byte = start_row * type_size;
- end_byte = end_row * type_size;
  }
  internal::check<ErrorCode::E_ASSERTION_FAILURE>(
- start_byte % type_size == 0,
- "start_byte {} is not a multiple of type size {}", start_byte, column->type());
+ start_byte % type_size == 0,
+ "start_byte {} is not a multiple of type size {}",
+ start_byte,
+ column.type()
+ );
  internal::check<ErrorCode::E_ASSERTION_FAILURE>(
- end_byte % type_size == 0,
- "start_byte {} is not a multiple of type size {}", end_byte, column->type());
+ end_byte % type_size == 0,
+ "start_byte {} is not a multiple of type size {}",
+ end_byte,
+ column.type()
+ );
+ return {start_byte, end_byte};
+}
+
+[[nodiscard]] static util::BitMagic truncate_sparse_map(
+ const util::BitMagic& input_sparse_map,
+ size_t start_row,
+ size_t end_row
+) {
+ // The output sparse map is the slice [start_row, end_row) of the input sparse map
+ // BitMagic doesn't have a method for this, so hand-roll it here
+ // Ctor parameter is the size
+ util::BitMagic output_sparse_map(end_row - start_row);
+ util::BitSet::bulk_insert_iterator inserter(output_sparse_map);
+ util::BitSetSizeType set_input_bit;
+ if (start_row == 0) {
+ // get_first can return 0 if no bits are set, but we checked earlier that input_sparse_map.size() > 0,
+ // and we do not have sparse maps with no bits set (except for the empty type)
+ set_input_bit = input_sparse_map.get_first();
+ if (set_input_bit < end_row) {
+ inserter = set_input_bit;
+ }
+ } else {
+ set_input_bit = input_sparse_map.get_next(start_row - 1);
+ // get_next returns 0 if no more bits are set
+ if (set_input_bit != 0 && set_input_bit < end_row) {
+ // Shift start_row elements to the left
+ inserter = set_input_bit - start_row;
+ }
+ }
+ do {
+ set_input_bit = input_sparse_map.get_next(set_input_bit);
+ if (set_input_bit != 0 && set_input_bit < end_row) {
+ inserter = set_input_bit - start_row;
+ }
+ } while (set_input_bit != 0);
+ inserter.flush();
+ return output_sparse_map;
+}
+
+std::shared_ptr<Column> Column::truncate(const std::shared_ptr<Column>& column, size_t start_row, size_t end_row) {
+ const auto [start_byte, end_byte] = column_start_end_bytes(*column, start_row, end_row);
  auto buffer = ::arcticdb::truncate(column->data_.buffer(), start_byte, end_byte);
  auto res = std::make_shared<Column>(column->type(), column->allow_sparse_, std::move(buffer));
  if (column->is_sparse()) {
- // The output sparse map is the slice [start_row, end_row) of the input sparse map
- // BitMagic doesn't have a method for this, so hand-roll it here
- // Ctor parameter is the size
- util::BitMagic output_sparse_map(end_row - start_row);
- util::BitSet::bulk_insert_iterator inserter(output_sparse_map);
- util::BitSetSizeType set_input_bit;
- if (start_row == 0) {
- // get_first can return 0 if no bits are set, but we checked earlier that input_sparse_map.size() > 0,
- // and we do not have sparse maps with no bits set
- set_input_bit = input_sparse_map.get_first();
- if (set_input_bit < end_row) {
- inserter = set_input_bit;
- }
- } else {
- set_input_bit = input_sparse_map.get_next(start_row - 1);
- // get_next returns 0 if no more bits are set
- if (set_input_bit != 0 && set_input_bit < end_row) {
- // Shift start_row elements to the left
- inserter = set_input_bit - start_row;
- }
- }
- do {
- set_input_bit = input_sparse_map.get_next(set_input_bit);
- if (set_input_bit != 0 && set_input_bit < end_row) {
- inserter = set_input_bit - start_row;
- }
- } while (set_input_bit != 0);
- inserter.flush();
+ util::BitMagic output_sparse_map = is_empty_type(column->type().data_type())
+ ? util::BitMagic{}
+ : truncate_sparse_map(column->sparse_map(), start_row, end_row);
  res->set_sparse_map(std::move(output_sparse_map));
  }
  res->set_row_data(end_row - (start_row + 1));
@@ -675,7 +698,7 @@ void Column::regenerate_offsets() const {
  }
 }
 
-[[nodiscard]] util::BitMagic& Column::sparse_map() {
+util::BitMagic& Column::sparse_map() {
  if(!sparse_map_)
  sparse_map_ = std::make_optional<util::BitMagic>(0);
 
@@ -687,4 +710,8 @@ const util::BitMagic& Column::sparse_map() const {
  return sparse_map_.value();
 }
 
+std::optional<util::BitMagic>& Column::opt_sparse_map() {
+ return sparse_map_;
+}
+
 } //namespace arcticdb