github-actions[bot] commented on code in PR #40157:
URL: https://github.com/apache/doris/pull/40157#discussion_r1736659782
##########
be/src/util/block_compression.cpp:
##########
@@ -740,70 +696,13 @@
size_t max_compressed_len(size_t len) override { return
snappy::MaxCompressedLength(len); }
};
-class HadoopSnappyBlockCompression : public SnappyBlockCompression {
-public:
- static HadoopSnappyBlockCompression* instance() {
- static HadoopSnappyBlockCompression s_instance;
- return &s_instance;
- }
- ~HadoopSnappyBlockCompression() override = default;
-
- // hadoop use block compression for snappy
- //
https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/SnappyCodec.cc
- Status compress(const Slice& input, faststring* output) override {
- // be same with hadop
https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/SnappyCodec.java
- size_t snappy_block_size = config::snappy_compression_block_size;
- size_t overhead = snappy_block_size / 6 + 32;
- size_t max_input_size = snappy_block_size - overhead;
-
- size_t data_len = input.size;
- char* data = input.data;
- std::vector<OwnedSlice> buffers;
- size_t out_len = 0;
-
- while (data_len > 0) {
- size_t input_size = std::min(data_len, max_input_size);
- Slice input_slice(data, input_size);
- faststring output_data;
- RETURN_IF_ERROR(SnappyBlockCompression::compress(input_slice,
&output_data));
- out_len += output_data.size();
- // the OwnedSlice will be moved here
- buffers.push_back(output_data.build());
- data += input_size;
- data_len -= input_size;
- }
-
- // hadoop block compression: umcompressed_length | compressed_length1
| compressed_data1 | compressed_length2 | compressed_data2 | ...
- size_t total_output_len = 4 + 4 * buffers.size() + out_len;
- output->resize(total_output_len);
- char* output_buffer = (char*)output->data();
- BigEndian::Store32(output_buffer, input.get_size());
- output_buffer += 4;
- for (const auto& buffer : buffers) {
- auto slice = buffer.slice();
- BigEndian::Store32(output_buffer, slice.get_size());
- output_buffer += 4;
- memcpy(output_buffer, slice.get_data(), slice.get_size());
- output_buffer += slice.get_size();
- }
-
- DCHECK_EQ(output_buffer - (char*)output->data(), total_output_len);
-
- return Status::OK();
- }
-
- Status decompress(const Slice& input, Slice* output) override {
- return Status::InternalError("unimplement:
SnappyHadoopBlockCompression::decompress");
- }
-};
-
class ZlibBlockCompression : public BlockCompressionCodec {
public:
static ZlibBlockCompression* instance() {
static ZlibBlockCompression s_instance;
return &s_instance;
}
- ~ZlibBlockCompression() override = default;
+ ~ZlibBlockCompression() {}
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final'
[modernize-use-override]
```suggestion
~ZlibBlockCompression() override {}
```
##########
be/src/util/block_compression.cpp:
##########
@@ -740,70 +696,13 @@
size_t max_compressed_len(size_t len) override { return
snappy::MaxCompressedLength(len); }
};
-class HadoopSnappyBlockCompression : public SnappyBlockCompression {
-public:
- static HadoopSnappyBlockCompression* instance() {
- static HadoopSnappyBlockCompression s_instance;
- return &s_instance;
- }
- ~HadoopSnappyBlockCompression() override = default;
-
- // hadoop use block compression for snappy
- //
https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-nativetask/src/main/native/src/codec/SnappyCodec.cc
- Status compress(const Slice& input, faststring* output) override {
- // be same with hadop
https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/SnappyCodec.java
- size_t snappy_block_size = config::snappy_compression_block_size;
- size_t overhead = snappy_block_size / 6 + 32;
- size_t max_input_size = snappy_block_size - overhead;
-
- size_t data_len = input.size;
- char* data = input.data;
- std::vector<OwnedSlice> buffers;
- size_t out_len = 0;
-
- while (data_len > 0) {
- size_t input_size = std::min(data_len, max_input_size);
- Slice input_slice(data, input_size);
- faststring output_data;
- RETURN_IF_ERROR(SnappyBlockCompression::compress(input_slice,
&output_data));
- out_len += output_data.size();
- // the OwnedSlice will be moved here
- buffers.push_back(output_data.build());
- data += input_size;
- data_len -= input_size;
- }
-
- // hadoop block compression: umcompressed_length | compressed_length1
| compressed_data1 | compressed_length2 | compressed_data2 | ...
- size_t total_output_len = 4 + 4 * buffers.size() + out_len;
- output->resize(total_output_len);
- char* output_buffer = (char*)output->data();
- BigEndian::Store32(output_buffer, input.get_size());
- output_buffer += 4;
- for (const auto& buffer : buffers) {
- auto slice = buffer.slice();
- BigEndian::Store32(output_buffer, slice.get_size());
- output_buffer += 4;
- memcpy(output_buffer, slice.get_data(), slice.get_size());
- output_buffer += slice.get_size();
- }
-
- DCHECK_EQ(output_buffer - (char*)output->data(), total_output_len);
-
- return Status::OK();
- }
-
- Status decompress(const Slice& input, Slice* output) override {
- return Status::InternalError("unimplement:
SnappyHadoopBlockCompression::decompress");
- }
-};
-
class ZlibBlockCompression : public BlockCompressionCodec {
public:
static ZlibBlockCompression* instance() {
static ZlibBlockCompression s_instance;
return &s_instance;
}
- ~ZlibBlockCompression() override = default;
+ ~ZlibBlockCompression() {}
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```suggestion
~ZlibBlockCompression() = default;
```
##########
be/src/vec/runtime/vcsv_transformer.h:
##########
@@ -21,15 +21,13 @@
#include <arrow/result.h>
#include <arrow/status.h>
#include <gen_cpp/DataSinks_types.h>
-#include <gen_cpp/PlanNodes_types.h>
#include <parquet/file_writer.h>
#include <parquet/properties.h>
#include <parquet/types.h>
+#include <stdint.h>
Review Comment:
warning: inclusion of deprecated C++ header 'stdint.h'; consider using
'cstdint' instead [modernize-deprecated-headers]
```suggestion
#include <cstdint>
```
##########
be/src/vec/runtime/vcsv_transformer.cpp:
##########
@@ -18,22 +18,39 @@
#include "vec/runtime/vcsv_transformer.h"
#include <glog/logging.h>
+#include <stdlib.h>
Review Comment:
warning: inclusion of deprecated C++ header 'stdlib.h'; consider using
'cstdlib' instead [modernize-deprecated-headers]
```suggestion
#include <cstdlib>
```
##########
be/src/util/block_compression.h:
##########
@@ -17,7 +17,6 @@
#pragma once
-#include <gen_cpp/PlanNodes_types.h>
#include <gen_cpp/parquet_types.h>
Review Comment:
warning: 'gen_cpp/parquet_types.h' file not found [clang-diagnostic-error]
```cpp
#include <gen_cpp/parquet_types.h>
^
```
##########
be/src/util/slice.h:
##########
@@ -373,6 +369,11 @@
OwnedSlice(uint8_t* _data, size_t size, size_t capacity)
: _slice(_data, size), _capacity(capacity) {}
+private:
+ // disable copy constructor and copy assignment
+ OwnedSlice(const OwnedSlice&) = delete;
+ void operator=(const OwnedSlice&) = delete;
Review Comment:
warning: deleted member function should be public
[modernize-use-equals-delete]
```cpp
void operator=(const OwnedSlice&) = delete;
^
```
##########
be/src/util/block_compression.cpp:
##########
@@ -88,7 +87,10 @@ Status BlockCompressionCodec::compress(const
std::vector<Slice>& inputs, size_t
}
bool BlockCompressionCodec::exceed_max_compress_len(size_t uncompressed_size) {
- return uncompressed_size > std::numeric_limits<int32_t>::max();
+ if (uncompressed_size > std::numeric_limits<int32_t>::max()) {
+ return true;
Review Comment:
warning: redundant boolean literal in conditional return statement
[readability-simplify-boolean-expr]
be/src/util/block_compression.cpp:89:
```diff
- if (uncompressed_size > std::numeric_limits<int32_t>::max()) {
- return true;
- }
- return false;
+ return uncompressed_size > std::numeric_limits<int32_t>::max();
```
##########
be/src/util/block_compression.cpp:
##########
@@ -705,7 +661,7 @@
static SnappyBlockCompression s_instance;
return &s_instance;
}
- ~SnappyBlockCompression() override = default;
+ ~SnappyBlockCompression() override {}
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```suggestion
~SnappyBlockCompression() override = default;
```
##########
be/src/util/slice.h:
##########
@@ -373,6 +369,11 @@ class OwnedSlice : private Allocator<false, false, false,
DefaultMemoryAllocator
OwnedSlice(uint8_t* _data, size_t size, size_t capacity)
: _slice(_data, size), _capacity(capacity) {}
+private:
Review Comment:
warning: redundant access specifier has the same accessibility as the
previous access specifier [readability-redundant-access-specifiers]
```suggestion
```
<details>
<summary>Additional context</summary>
**be/src/util/slice.h:364:** previously declared here
```cpp
private:
^
```
</details>
##########
be/src/vec/runtime/vcsv_transformer.cpp:
##########
@@ -18,22 +18,39 @@
#include "vec/runtime/vcsv_transformer.h"
#include <glog/logging.h>
+#include <stdlib.h>
+#include <string.h>
Review Comment:
warning: inclusion of deprecated C++ header 'string.h'; consider using
'cstring' instead [modernize-deprecated-headers]
```suggestion
#include <cstring>
```
##########
be/src/util/slice.h:
##########
@@ -373,6 +369,11 @@
OwnedSlice(uint8_t* _data, size_t size, size_t capacity)
: _slice(_data, size), _capacity(capacity) {}
+private:
+ // disable copy constructor and copy assignment
+ OwnedSlice(const OwnedSlice&) = delete;
Review Comment:
warning: deleted member function should be public
[modernize-use-equals-delete]
```cpp
OwnedSlice(const OwnedSlice&) = delete;
^
```
--
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]
