From 96c31ea3eafe861a1390c3890833777871bf3e56 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 15 Jul 2022 16:45:08 +0800 Subject: [PATCH 1/3] [Bug] fix wrong null flag --- be/src/vec/olap/olap_data_convertor.cpp | 41 +++++++++++++------------ be/src/vec/olap/olap_data_convertor.h | 2 +- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/be/src/vec/olap/olap_data_convertor.cpp b/be/src/vec/olap/olap_data_convertor.cpp index 2bf619dece742b..d72c12651b161d 100644 --- a/be/src/vec/olap/olap_data_convertor.cpp +++ b/be/src/vec/olap/olap_data_convertor.cpp @@ -148,7 +148,6 @@ void OlapBlockDataConvertor::OlapColumnDataConvertorBase::set_source_column( auto nullable_column = assert_cast(_typed_column.column.get()); _nullmap = nullable_column->get_null_map_data().data(); - _nullmap += row_pos; } } @@ -157,9 +156,11 @@ void OlapBlockDataConvertor::OlapColumnDataConvertorBase::clear_source_column() _typed_column.column = nullptr; } +// This should be called only in SegmentWriter. If you want to access nullmap in Convertor, +// use `_nullmap` directly. const UInt8* OlapBlockDataConvertor::OlapColumnDataConvertorBase::get_nullmap() const { assert(_typed_column.column); - return _nullmap; + return _nullmap + _row_pos; } // class OlapBlockDataConvertor::OlapColumnDataConvertorObject @@ -205,7 +206,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorBitMap::convert_to_olap() size_t total_size = 0; if (_nullmap) { - const UInt8* nullmap_cur = _nullmap; + const UInt8* nullmap_cur = _nullmap + _row_pos; while (bitmap_value_cur != bitmap_value_end) { if (!*nullmap_cur) { total_size += bitmap_value_cur->getSizeInBytes(); @@ -226,7 +227,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorBitMap::convert_to_olap() char* raw_data = _raw_data.data(); Slice* slice = _slice.data(); if (_nullmap) { - const UInt8* nullmap_cur = _nullmap; + const UInt8* nullmap_cur = _nullmap + _row_pos; while (bitmap_value_cur != bitmap_value_end) { if (!*nullmap_cur) { slice_size = bitmap_value_cur->getSizeInBytes(); @@ -244,7 +245,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorBitMap::convert_to_olap() ++nullmap_cur; ++bitmap_value_cur; } - assert(nullmap_cur == _nullmap + _num_rows && slice == _slice.get_end_ptr()); + assert(nullmap_cur == _nullmap + _row_pos + _num_rows && slice == _slice.get_end_ptr()); } else { while (bitmap_value_cur != bitmap_value_end) { slice_size = bitmap_value_cur->getSizeInBytes(); @@ -281,7 +282,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorHLL::convert_to_olap() { size_t total_size = 0; if (_nullmap) { - const UInt8* nullmap_cur = _nullmap; + const UInt8* nullmap_cur = _nullmap + _row_pos; while (hll_value_cur != hll_value_end) { if (!*nullmap_cur) { total_size += hll_value_cur->max_serialized_size(); @@ -303,7 +304,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorHLL::convert_to_olap() { hll_value_cur = hll_value; if (_nullmap) { - const UInt8* nullmap_cur = _nullmap; + const UInt8* nullmap_cur = _nullmap + _row_pos; while (hll_value_cur != hll_value_end) { if (!*nullmap_cur) { slice_size = hll_value_cur->serialize((uint8_t*)raw_data); @@ -320,7 +321,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorHLL::convert_to_olap() { ++nullmap_cur; ++hll_value_cur; } - assert(nullmap_cur == _nullmap + _num_rows && slice == _slice.get_end_ptr()); + assert(nullmap_cur == _nullmap + _row_pos + _num_rows && slice == _slice.get_end_ptr()); } else { while (hll_value_cur != hll_value_end) { slice_size = hll_value_cur->serialize((uint8_t*)raw_data); @@ -382,7 +383,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorChar::convert_to_olap() { } for (size_t i = 0; i < _num_rows; i++) { - if (!_nullmap || !_nullmap[i]) { + if (!_nullmap || !_nullmap[i + _row_pos]) { _slice[i] = column_string->get_data_at(i + _row_pos).to_slice(); DCHECK(_slice[i].size == _length) << "char type data length not equal to schema, schema=" << _length @@ -440,7 +441,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorVarChar::convert_to_olap() Slice* slice = _slice.data(); size_t string_offset = *(offset_cur - 1); if (_nullmap) { - const UInt8* nullmap_cur = _nullmap; + const UInt8* nullmap_cur = _nullmap + _row_pos; while (offset_cur != offset_end) { if (!*nullmap_cur) { slice->data = const_cast(char_data + string_offset); @@ -461,7 +462,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorVarChar::convert_to_olap() ++slice; ++offset_cur; } - assert(nullmap_cur == _nullmap + _num_rows && slice == _slice.get_end_ptr()); + assert(nullmap_cur == _nullmap + _row_pos + _num_rows && slice == _slice.get_end_ptr()); } else { while (offset_cur != offset_end) { slice->data = const_cast(char_data + string_offset); @@ -513,7 +514,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorDate::convert_to_olap() { const DateV2Value* datetime_end = datetime_cur + _num_rows; uint24_t* value = _values.data(); if (_nullmap) { - const UInt8* nullmap_cur = _nullmap; + const UInt8* nullmap_cur = _nullmap + _row_pos; while (datetime_cur != datetime_end) { if (!*nullmap_cur) { *value = datetime_cur->to_olap_date(); @@ -524,7 +525,8 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorDate::convert_to_olap() { ++datetime_cur; ++nullmap_cur; } - assert(nullmap_cur == _nullmap + _num_rows && value == _values.get_end_ptr()); + assert(nullmap_cur == _nullmap + _row_pos + _num_rows && + value == _values.get_end_ptr()); } else { while (datetime_cur != datetime_end) { *value = datetime_cur->to_olap_date(); @@ -553,7 +555,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorDate::convert_to_olap() { const VecDateTimeValue* datetime_end = datetime_cur + _num_rows; uint24_t* value = _values.data(); if (_nullmap) { - const UInt8* nullmap_cur = _nullmap; + const UInt8* nullmap_cur = _nullmap + _row_pos; while (datetime_cur != datetime_end) { if (!*nullmap_cur) { *value = datetime_cur->to_olap_date(); @@ -564,7 +566,8 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorDate::convert_to_olap() { ++datetime_cur; ++nullmap_cur; } - assert(nullmap_cur == _nullmap + _num_rows && value == _values.get_end_ptr()); + assert(nullmap_cur == _nullmap + _row_pos + _num_rows && + value == _values.get_end_ptr()); } else { while (datetime_cur != datetime_end) { *value = datetime_cur->to_olap_date(); @@ -597,7 +600,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorDateTime::convert_to_olap( const VecDateTimeValue* datetime_end = datetime_cur + _num_rows; uint64_t* value = _values.data(); if (_nullmap) { - const UInt8* nullmap_cur = _nullmap; + const UInt8* nullmap_cur = _nullmap + _row_pos; while (datetime_cur != datetime_end) { if (!*nullmap_cur) { *value = datetime_cur->to_olap_datetime(); @@ -608,7 +611,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorDateTime::convert_to_olap( ++datetime_cur; ++nullmap_cur; } - assert(nullmap_cur == _nullmap + _num_rows && value == _values.get_end_ptr()); + assert(nullmap_cur == _nullmap + _row_pos + _num_rows && value == _values.get_end_ptr()); } else { while (datetime_cur != datetime_end) { *value = datetime_cur->to_olap_datetime(); @@ -652,7 +655,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorDecimal::convert_to_olap() ++decimal_cur; ++nullmap_cur; } - assert(nullmap_cur == _nullmap + _num_rows && value == _values.get_end_ptr()); + assert(nullmap_cur == _nullmap + _row_pos + _num_rows && value == _values.get_end_ptr()); } else { while (decimal_cur != decimal_end) { value->integer = decimal_cur->int_value(); @@ -713,7 +716,7 @@ Status OlapBlockDataConvertor::OlapColumnDataConvertorArray::convert_to_olap( for (size_t i = 0; i < _num_rows; ++i, ++collection_value) { int64_t cur_pos = _row_pos + i; int64_t prev_pos = cur_pos - 1; - if (_nullmap && _nullmap[cur_pos - _row_pos]) { + if (_nullmap && _nullmap[cur_pos]) { continue; } auto offset = offsets[prev_pos]; diff --git a/be/src/vec/olap/olap_data_convertor.h b/be/src/vec/olap/olap_data_convertor.h index 37bd4b1ec3eca4..2af6bd505f7d49 100644 --- a/be/src/vec/olap/olap_data_convertor.h +++ b/be/src/vec/olap/olap_data_convertor.h @@ -298,7 +298,7 @@ class OlapBlockDataConvertor { const VecDateTimeValue* datetime_end = datetime_cur + _num_rows; uint32_t* value = const_cast(values_); if (_nullmap) { - const UInt8* nullmap_cur = _nullmap; + const UInt8* nullmap_cur = _nullmap + _row_pos; while (datetime_cur != datetime_end) { if (!*nullmap_cur) { *value = datetime_cur->to_date_v2(); From 0756a6e050b86d5cda451d487886c3a1a06bb53d Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 15 Jul 2022 21:59:41 +0800 Subject: [PATCH 2/3] update --- be/src/vec/olap/olap_data_convertor.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/be/src/vec/olap/olap_data_convertor.cpp b/be/src/vec/olap/olap_data_convertor.cpp index d72c12651b161d..0c3d3d14300de9 100644 --- a/be/src/vec/olap/olap_data_convertor.cpp +++ b/be/src/vec/olap/olap_data_convertor.cpp @@ -412,7 +412,6 @@ const void* OlapBlockDataConvertor::OlapColumnDataConvertorVarChar::get_data() c const void* OlapBlockDataConvertor::OlapColumnDataConvertorVarChar::get_data_at( size_t offset) const { - assert(offset < _num_rows && _num_rows == _slice.size()); UInt8 null_flag = 0; if (_nullmap) { null_flag = _nullmap[offset]; From 829230177de3d56b0ff4c29d3c6f5f5af4de50fb Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 15 Jul 2022 22:01:33 +0800 Subject: [PATCH 3/3] update --- be/src/vec/olap/olap_data_convertor.cpp | 2 -- be/src/vec/olap/olap_data_convertor.h | 1 - 2 files changed, 3 deletions(-) diff --git a/be/src/vec/olap/olap_data_convertor.cpp b/be/src/vec/olap/olap_data_convertor.cpp index 0c3d3d14300de9..a7238c64f394c3 100644 --- a/be/src/vec/olap/olap_data_convertor.cpp +++ b/be/src/vec/olap/olap_data_convertor.cpp @@ -178,7 +178,6 @@ const void* OlapBlockDataConvertor::OlapColumnDataConvertorObject::get_data() co const void* OlapBlockDataConvertor::OlapColumnDataConvertorObject::get_data_at( size_t offset) const { - assert(offset < _num_rows && _num_rows == _slice.size()); UInt8 null_flag = 0; if (_nullmap) { null_flag = _nullmap[offset]; @@ -356,7 +355,6 @@ const void* OlapBlockDataConvertor::OlapColumnDataConvertorChar::get_data() cons } const void* OlapBlockDataConvertor::OlapColumnDataConvertorChar::get_data_at(size_t offset) const { - assert(offset < _num_rows && _num_rows == _slice.size()); UInt8 null_flag = 0; if (_nullmap) { null_flag = _nullmap[offset]; diff --git a/be/src/vec/olap/olap_data_convertor.h b/be/src/vec/olap/olap_data_convertor.h index 2af6bd505f7d49..7b844d04f6b370 100644 --- a/be/src/vec/olap/olap_data_convertor.h +++ b/be/src/vec/olap/olap_data_convertor.h @@ -178,7 +178,6 @@ class OlapBlockDataConvertor { } const void* get_data() const override { return _values.data(); } const void* get_data_at(size_t offset) const override { - assert(offset < _num_rows && _num_rows == _values.size()); UInt8 null_flag = 0; if (_nullmap) { null_flag = _nullmap[offset];