github-actions[bot] commented on code in PR #17112:
URL: https://github.com/apache/doris/pull/17112#discussion_r1124184221


##########
be/src/util/bit_stream_utils.inline.h:
##########
@@ -227,12 +275,14 @@ inline int BatchedBitReader::UnpackBatch(int bit_width, 
int num_values, T* v) {
 
 inline bool BatchedBitReader::SkipBatch(int bit_width, int num_values_to_skip) 
{
     DCHECK(buffer_pos_ != nullptr);
-    DCHECK_GT(bit_width, 0);
+    DCHECK_GE(bit_width, 0);
     DCHECK_LE(bit_width, MAX_BITWIDTH);
-    DCHECK_GT(num_values_to_skip, 0);
+    DCHECK_GE(num_values_to_skip, 0);
 
     int skip_bytes = BitUtil::RoundUpNumBytes(bit_width * num_values_to_skip);
-    if (skip_bytes > buffer_end_ - buffer_pos_) return false;
+    if (skip_bytes > buffer_end_ - buffer_pos_ + 1) {

Review Comment:
   warning: statement should be inside braces 
[readability-braces-around-statements]
   
   ```suggestion
       if (skip_bytes > buffer_end_ - buffer_pos_ + 1) { return false;
   }
   ```
   



##########
be/src/vec/exec/format/parquet/delta_bit_pack_decoder.h:
##########
@@ -0,0 +1,381 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include "util/bit_stream_utils.h"
+#include "vec/columns/column.h"
+#include "vec/exec/format/parquet/decoder.h"
+
+namespace doris::vectorized {
+/**
+ *   Format
+ *      [header] [block 1] [block 2] ... [block N]
+ *   Header
+ *      [block size] [_mini_blocks_per_block] [_total_value_count] [first 
value]
+ *   Block
+ *      [min delta] [list of bitwidths of the mini blocks] [miniblocks]
+ */
+template <typename T>
+class DeltaBitPackDecoder final : public Decoder {
+public:
+    using UT = std::make_unsigned_t<T>;
+
+    DeltaBitPackDecoder(const tparquet::Type::type& physical_type)
+            : _physical_type(physical_type) {}
+    ~DeltaBitPackDecoder() override = default;
+    Status decode_values(MutableColumnPtr& doris_column, DataTypePtr& 
data_type,
+                         ColumnSelectVector& select_vector) override {
+        size_t non_null_size = select_vector.num_values() - 
select_vector.num_nulls();
+        // decode values
+        _values.resize(non_null_size);
+        int decoded_count = 0;
+        RETURN_IF_ERROR(_get_internal(_values.data(), non_null_size, 
&decoded_count));
+
+        // set value
+        TypeIndex logical_type = remove_nullable(data_type)->get_type_id();
+        switch (logical_type) {
+#define DISPATCH(NUMERIC_TYPE, CPP_NUMERIC_TYPE, PHYSICAL_TYPE)                
    \
+    case NUMERIC_TYPE:                                                         
    \
+        if constexpr (std::is_same_v<T, PHYSICAL_TYPE>) {                      
    \
+            return _decode_numeric<CPP_NUMERIC_TYPE>(doris_column, 
select_vector); \
+        }
+            FOR_LOGICAL_NUMERIC_TYPES(DISPATCH)
+#undef DISPATCH
+        case TypeIndex::DateTime:
+            return _decode_datetime64<VecDateTimeValue, Int64>(doris_column, 
select_vector);
+        case TypeIndex::DateTimeV2:
+            return _decode_datetime64<DateV2Value<DateTimeV2ValueType>, 
UInt64>(doris_column,
+                                                                               
 select_vector);
+        default:
+            break;
+        }
+        return Status::InvalidArgument(
+                "Can't decode parquet physical type {} to doris logical type 
{}",
+                tparquet::to_string(_physical_type), 
getTypeName(logical_type));
+    }
+    template <typename Numeric>
+    Status _decode_numeric(MutableColumnPtr& doris_column, ColumnSelectVector& 
select_vector) {
+        auto& column_data = 
static_cast<ColumnVector<Numeric>&>(*doris_column).get_data();
+        size_t data_index = column_data.size();
+        column_data.resize(data_index + select_vector.num_values() - 
select_vector.num_filtered());
+        ColumnSelectVector::DataReadType read_type;
+        while (size_t run_length = select_vector.get_next_run(&read_type)) {
+            switch (read_type) {
+            case ColumnSelectVector::CONTENT: {
+                for (size_t i = 0; i < run_length; ++i) {
+                    column_data[data_index++] = 
static_cast<Numeric>(_values[i++]);
+                }
+                break;
+            }
+            case ColumnSelectVector::NULL_DATA: {
+                data_index += run_length;
+                break;
+            }
+            case ColumnSelectVector::FILTERED_CONTENT: {
+                data_index += run_length;
+                break;
+            }
+            case ColumnSelectVector::FILTERED_NULL: {
+                // do nothing
+                break;
+            }
+            }
+        }
+        return Status::OK();
+    }
+
+    template <typename CppType, typename ColumnType>
+    Status _decode_datetime64(MutableColumnPtr& doris_column, 
ColumnSelectVector& select_vector) {
+        auto& column_data = 
static_cast<ColumnVector<ColumnType>&>(*doris_column).get_data();
+        size_t data_index = column_data.size();
+        column_data.resize(data_index + select_vector.num_values() - 
select_vector.num_filtered());
+        ColumnSelectVector::DataReadType read_type;
+        while (size_t run_length = select_vector.get_next_run(&read_type)) {
+            switch (read_type) {
+            case ColumnSelectVector::CONTENT: {
+                for (size_t i = 0; i < run_length; ++i) {
+                    int64_t date_value = _values[i++];
+                    auto& v = 
reinterpret_cast<CppType&>(column_data[data_index++]);
+                    v.from_unixtime(date_value / _decode_params->second_mask, 
*_decode_params->ctz);
+                    if constexpr (std::is_same_v<CppType, 
DateV2Value<DateTimeV2ValueType>>) {
+                        // nanoseconds will be ignored.
+                        v.set_microsecond((date_value % 
_decode_params->second_mask) *
+                                          _decode_params->scale_to_nano_factor 
/ 1000);
+                    }
+                }
+                break;
+            }
+            case ColumnSelectVector::NULL_DATA: {
+                data_index += run_length;
+                break;
+            }
+            case ColumnSelectVector::FILTERED_CONTENT: {
+                data_index += run_length;
+                break;
+            }
+            case ColumnSelectVector::FILTERED_NULL: {
+                // do nothing
+                break;
+            }
+            }
+        }
+        return Status::OK();
+    }
+
+    Status decode(T* buffer, int num_values, int* out_num_values) {
+        return _get_internal(buffer, num_values, out_num_values);
+    }
+
+    int valid_values_count() {
+        // _total_value_count in header ignores of null values
+        return static_cast<int>(_total_value_count);
+    }
+
+    Status skip_values(size_t num_values) override {
+        return Status::OK();
+    }
+
+    void set_data(Slice* slice) override {
+        _bit_reader.reset(new BatchedBitReader((const uint8_t*)slice->data, 
slice->size));
+        _init_header();
+        _data = slice;
+        _offset = 0;
+    }
+
+    // Set BitReader which is already initialized by 
DeltaLengthByteArrayDecoder or
+    // DeltaByteArrayDecoder
+    void set_decoder(std::shared_ptr<BatchedBitReader> bit_reader) {
+        _bit_reader = std::move(bit_reader);
+        _init_header();
+    }
+
+private:
+    static constexpr int kMaxDeltaBitWidth = static_cast<int>(sizeof(T) * 8);
+    Status _init_header();
+    Status _init_block();
+    Status _init_mini_block(int bit_width);
+    Status _get_internal(T* buffer, int max_values, int* out_num_values);
+
+    tparquet::Type::type _physical_type;
+    std::vector<T> _values;
+
+    std::shared_ptr<BatchedBitReader> _bit_reader;
+    uint32_t _values_per_block;
+    uint32_t _mini_blocks_per_block;
+    uint32_t _values_per_mini_block;
+    uint32_t _total_value_count;
+
+    T _min_delta;
+    T _last_value;
+
+    uint32_t _mini_block_idx;
+    std::vector<uint8_t> _delta_bit_widths;
+    int _delta_bit_width;
+    // If the page doesn't contain any block, `_block_initialized` will
+    // always be false. Otherwise, it will be true when first block 
initialized.
+    bool _block_initialized;
+
+    uint32_t _total_values_remaining;
+    // Remaining values in current mini block. If the current block is the 
last mini block,
+    // _values_remaining_current_mini_block may greater than 
_total_values_remaining.
+    uint32_t _values_remaining_current_mini_block;
+};
+template class DeltaBitPackDecoder<int32_t>;
+template class DeltaBitPackDecoder<int64_t>;
+
+class DeltaLengthByteArrayDecoder final : public Decoder {
+public:
+    explicit DeltaLengthByteArrayDecoder(const tparquet::Type::type& 
physical_type)
+            : _len_decoder(physical_type), _buffered_length(0), 
_buffered_data(0) {}
+
+    Status decode_values(MutableColumnPtr& doris_column, DataTypePtr& 
data_type,
+                         ColumnSelectVector& select_vector) override {
+        size_t num_values = select_vector.num_values();
+        size_t null_count = select_vector.num_nulls();
+        std::vector<Slice> values(num_values - null_count);
+        int num_valid_values;
+        RETURN_IF_ERROR(_get_internal(values.data(), num_values - null_count, 
&num_valid_values));
+
+        if (PREDICT_FALSE(num_values - null_count != num_valid_values)) {
+            return Status::EndOfFile("Expected to decode ", num_values - 
null_count,
+                                     " values, but decoded ", 
num_valid_values, " values.");
+        }
+
+        return Status::OK();
+    }
+
+    Status decode(Slice* buffer, int num_values, int* out_num_values) {
+        return _get_internal(buffer, num_values, out_num_values);
+    }
+
+    void set_data(Slice* slice) override {
+        if (slice->size == 0) {
+            return;
+        }
+        _bit_reader = std::make_shared<BatchedBitReader>((const 
uint8_t*)slice->data, slice->size);
+        _data = slice;
+        _offset = 0;
+        _decode_lengths();
+    }
+
+    void set_decoder(std::shared_ptr<BatchedBitReader> bit_reader) {
+        _bit_reader = std::move(bit_reader);
+        _decode_lengths();
+    }
+
+    Status skip_values(size_t num_values) override { return Status::OK(); }
+
+private:
+    // Decode all the encoded lengths. The decoder_ will be at the start of 
the encoded data
+    // after that.
+    void _decode_lengths();
+    Status _get_internal(Slice* buffer, int max_values, int* out_num_values);
+
+    std::shared_ptr<BatchedBitReader> _bit_reader;
+    DeltaBitPackDecoder<int32_t> _len_decoder;
+
+    int _num_valid_values;
+    uint32_t _length_idx;
+    std::vector<char> _buffered_length;
+    std::vector<char> _buffered_data;
+};
+
+class DeltaByteArrayDecoder : public Decoder {
+public:
+    explicit DeltaByteArrayDecoder(const tparquet::Type::type& physical_type)
+            : prefix_len_decoder_(physical_type),
+              suffix_decoder_(physical_type),
+              last_value_in_previous_page_(""),

Review Comment:
   warning: redundant string initialization [readability-redundant-string-init]
   
   ```suggestion
                 ,
   ```
   



##########
be/src/vec/exec/format/parquet/delta_bit_pack_decoder.h:
##########
@@ -0,0 +1,381 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include "util/bit_stream_utils.h"
+#include "vec/columns/column.h"
+#include "vec/exec/format/parquet/decoder.h"
+
+namespace doris::vectorized {
+/**
+ *   Format
+ *      [header] [block 1] [block 2] ... [block N]
+ *   Header
+ *      [block size] [_mini_blocks_per_block] [_total_value_count] [first 
value]
+ *   Block
+ *      [min delta] [list of bitwidths of the mini blocks] [miniblocks]
+ */
+template <typename T>
+class DeltaBitPackDecoder final : public Decoder {
+public:
+    using UT = std::make_unsigned_t<T>;
+
+    DeltaBitPackDecoder(const tparquet::Type::type& physical_type)
+            : _physical_type(physical_type) {}
+    ~DeltaBitPackDecoder() override = default;
+    Status decode_values(MutableColumnPtr& doris_column, DataTypePtr& 
data_type,
+                         ColumnSelectVector& select_vector) override {
+        size_t non_null_size = select_vector.num_values() - 
select_vector.num_nulls();
+        // decode values
+        _values.resize(non_null_size);
+        int decoded_count = 0;
+        RETURN_IF_ERROR(_get_internal(_values.data(), non_null_size, 
&decoded_count));
+
+        // set value
+        TypeIndex logical_type = remove_nullable(data_type)->get_type_id();
+        switch (logical_type) {
+#define DISPATCH(NUMERIC_TYPE, CPP_NUMERIC_TYPE, PHYSICAL_TYPE)                
    \
+    case NUMERIC_TYPE:                                                         
    \
+        if constexpr (std::is_same_v<T, PHYSICAL_TYPE>) {                      
    \
+            return _decode_numeric<CPP_NUMERIC_TYPE>(doris_column, 
select_vector); \
+        }
+            FOR_LOGICAL_NUMERIC_TYPES(DISPATCH)
+#undef DISPATCH
+        case TypeIndex::DateTime:
+            return _decode_datetime64<VecDateTimeValue, Int64>(doris_column, 
select_vector);
+        case TypeIndex::DateTimeV2:
+            return _decode_datetime64<DateV2Value<DateTimeV2ValueType>, 
UInt64>(doris_column,
+                                                                               
 select_vector);
+        default:
+            break;
+        }
+        return Status::InvalidArgument(
+                "Can't decode parquet physical type {} to doris logical type 
{}",
+                tparquet::to_string(_physical_type), 
getTypeName(logical_type));
+    }
+    template <typename Numeric>
+    Status _decode_numeric(MutableColumnPtr& doris_column, ColumnSelectVector& 
select_vector) {
+        auto& column_data = 
static_cast<ColumnVector<Numeric>&>(*doris_column).get_data();
+        size_t data_index = column_data.size();
+        column_data.resize(data_index + select_vector.num_values() - 
select_vector.num_filtered());
+        ColumnSelectVector::DataReadType read_type;
+        while (size_t run_length = select_vector.get_next_run(&read_type)) {
+            switch (read_type) {
+            case ColumnSelectVector::CONTENT: {
+                for (size_t i = 0; i < run_length; ++i) {
+                    column_data[data_index++] = 
static_cast<Numeric>(_values[i++]);
+                }
+                break;
+            }
+            case ColumnSelectVector::NULL_DATA: {
+                data_index += run_length;
+                break;
+            }
+            case ColumnSelectVector::FILTERED_CONTENT: {
+                data_index += run_length;
+                break;
+            }
+            case ColumnSelectVector::FILTERED_NULL: {
+                // do nothing
+                break;
+            }
+            }
+        }
+        return Status::OK();
+    }
+
+    template <typename CppType, typename ColumnType>
+    Status _decode_datetime64(MutableColumnPtr& doris_column, 
ColumnSelectVector& select_vector) {
+        auto& column_data = 
static_cast<ColumnVector<ColumnType>&>(*doris_column).get_data();
+        size_t data_index = column_data.size();
+        column_data.resize(data_index + select_vector.num_values() - 
select_vector.num_filtered());
+        ColumnSelectVector::DataReadType read_type;
+        while (size_t run_length = select_vector.get_next_run(&read_type)) {
+            switch (read_type) {
+            case ColumnSelectVector::CONTENT: {
+                for (size_t i = 0; i < run_length; ++i) {
+                    int64_t date_value = _values[i++];
+                    auto& v = 
reinterpret_cast<CppType&>(column_data[data_index++]);
+                    v.from_unixtime(date_value / _decode_params->second_mask, 
*_decode_params->ctz);
+                    if constexpr (std::is_same_v<CppType, 
DateV2Value<DateTimeV2ValueType>>) {
+                        // nanoseconds will be ignored.
+                        v.set_microsecond((date_value % 
_decode_params->second_mask) *
+                                          _decode_params->scale_to_nano_factor 
/ 1000);
+                    }
+                }
+                break;
+            }
+            case ColumnSelectVector::NULL_DATA: {
+                data_index += run_length;
+                break;
+            }
+            case ColumnSelectVector::FILTERED_CONTENT: {
+                data_index += run_length;
+                break;
+            }
+            case ColumnSelectVector::FILTERED_NULL: {
+                // do nothing
+                break;
+            }
+            }
+        }
+        return Status::OK();
+    }
+
+    Status decode(T* buffer, int num_values, int* out_num_values) {
+        return _get_internal(buffer, num_values, out_num_values);
+    }
+
+    int valid_values_count() {
+        // _total_value_count in header ignores of null values
+        return static_cast<int>(_total_value_count);
+    }
+
+    Status skip_values(size_t num_values) override {
+        return Status::OK();
+    }
+
+    void set_data(Slice* slice) override {
+        _bit_reader.reset(new BatchedBitReader((const uint8_t*)slice->data, 
slice->size));
+        _init_header();
+        _data = slice;
+        _offset = 0;
+    }
+
+    // Set BitReader which is already initialized by 
DeltaLengthByteArrayDecoder or
+    // DeltaByteArrayDecoder
+    void set_decoder(std::shared_ptr<BatchedBitReader> bit_reader) {
+        _bit_reader = std::move(bit_reader);
+        _init_header();
+    }
+
+private:
+    static constexpr int kMaxDeltaBitWidth = static_cast<int>(sizeof(T) * 8);
+    Status _init_header();
+    Status _init_block();
+    Status _init_mini_block(int bit_width);
+    Status _get_internal(T* buffer, int max_values, int* out_num_values);
+
+    tparquet::Type::type _physical_type;
+    std::vector<T> _values;
+
+    std::shared_ptr<BatchedBitReader> _bit_reader;
+    uint32_t _values_per_block;
+    uint32_t _mini_blocks_per_block;
+    uint32_t _values_per_mini_block;
+    uint32_t _total_value_count;
+
+    T _min_delta;
+    T _last_value;
+
+    uint32_t _mini_block_idx;
+    std::vector<uint8_t> _delta_bit_widths;
+    int _delta_bit_width;
+    // If the page doesn't contain any block, `_block_initialized` will
+    // always be false. Otherwise, it will be true when first block 
initialized.
+    bool _block_initialized;
+
+    uint32_t _total_values_remaining;
+    // Remaining values in current mini block. If the current block is the 
last mini block,
+    // _values_remaining_current_mini_block may greater than 
_total_values_remaining.
+    uint32_t _values_remaining_current_mini_block;
+};
+template class DeltaBitPackDecoder<int32_t>;
+template class DeltaBitPackDecoder<int64_t>;
+
+class DeltaLengthByteArrayDecoder final : public Decoder {
+public:
+    explicit DeltaLengthByteArrayDecoder(const tparquet::Type::type& 
physical_type)
+            : _len_decoder(physical_type), _buffered_length(0), 
_buffered_data(0) {}
+
+    Status decode_values(MutableColumnPtr& doris_column, DataTypePtr& 
data_type,
+                         ColumnSelectVector& select_vector) override {
+        size_t num_values = select_vector.num_values();
+        size_t null_count = select_vector.num_nulls();
+        std::vector<Slice> values(num_values - null_count);
+        int num_valid_values;
+        RETURN_IF_ERROR(_get_internal(values.data(), num_values - null_count, 
&num_valid_values));
+
+        if (PREDICT_FALSE(num_values - null_count != num_valid_values)) {
+            return Status::EndOfFile("Expected to decode ", num_values - 
null_count,
+                                     " values, but decoded ", 
num_valid_values, " values.");
+        }
+
+        return Status::OK();
+    }
+
+    Status decode(Slice* buffer, int num_values, int* out_num_values) {
+        return _get_internal(buffer, num_values, out_num_values);
+    }
+
+    void set_data(Slice* slice) override {
+        if (slice->size == 0) {
+            return;
+        }
+        _bit_reader = std::make_shared<BatchedBitReader>((const 
uint8_t*)slice->data, slice->size);
+        _data = slice;
+        _offset = 0;
+        _decode_lengths();
+    }
+
+    void set_decoder(std::shared_ptr<BatchedBitReader> bit_reader) {
+        _bit_reader = std::move(bit_reader);
+        _decode_lengths();
+    }
+
+    Status skip_values(size_t num_values) override { return Status::OK(); }
+
+private:
+    // Decode all the encoded lengths. The decoder_ will be at the start of 
the encoded data
+    // after that.
+    void _decode_lengths();
+    Status _get_internal(Slice* buffer, int max_values, int* out_num_values);
+
+    std::shared_ptr<BatchedBitReader> _bit_reader;
+    DeltaBitPackDecoder<int32_t> _len_decoder;
+
+    int _num_valid_values;
+    uint32_t _length_idx;
+    std::vector<char> _buffered_length;
+    std::vector<char> _buffered_data;
+};
+
+class DeltaByteArrayDecoder : public Decoder {
+public:
+    explicit DeltaByteArrayDecoder(const tparquet::Type::type& physical_type)
+            : prefix_len_decoder_(physical_type),
+              suffix_decoder_(physical_type),
+              last_value_in_previous_page_(""),
+              buffered_prefix_length_(0),
+              buffered_data_(0) {}
+
+    void set_data(Slice* slice) override {
+        _bit_reader = std::make_shared<BatchedBitReader>((const 
uint8_t*)slice->data, slice->size);
+        prefix_len_decoder_.set_decoder(_bit_reader);
+
+        // get the number of encoded prefix lengths
+        int num_prefix = prefix_len_decoder_.valid_values_count();
+        // call prefix_len_decoder_.Decode to decode all the prefix lengths.
+        // all the prefix lengths are buffered in buffered_prefix_length_.
+        buffered_prefix_length_.resize(num_prefix * sizeof(int32_t));
+        int ret;
+        
prefix_len_decoder_.decode(reinterpret_cast<int32_t*>(buffered_prefix_length_.data()),
+                                   num_prefix, &ret);
+        DCHECK_EQ(ret, num_prefix);
+        prefix_len_offset_ = 0;
+        num_valid_values_ = num_prefix;
+
+        // at this time, the decoder_ will be at the start of the encoded 
suffix data.
+        suffix_decoder_.set_decoder(_bit_reader);
+
+        // TODO: read corrupted files written with bug(PARQUET-246). 
last_value_ should be set
+        // to last_value_in_previous_page_ when decoding a new page(except the 
first page)
+        last_value_ = "";
+    }
+
+    Status decode(Slice* buffer, int num_values, int* out_num_values) {
+        RETURN_IF_ERROR(_get_internal(buffer, num_values, out_num_values));
+    }

Review Comment:
   warning: non-void function does not return a value in all control paths 
[clang-diagnostic-return-type]
   ```cpp
       }
       ^
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to