You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2021/03/27 11:00:16 UTC
[incubator-doris] branch master updated: [Refactor] make uint24_t,
OLAPIndexFixedHeader as a POD type (#5559)
This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git
The following commit(s) were added to refs/heads/master by this push:
new a56e7e2 [Refactor] make uint24_t,OLAPIndexFixedHeader as a POD type (#5559)
a56e7e2 is described below
commit a56e7e2192a6642af41b2135627569cf5803c392
Author: stdpain <34...@users.noreply.github.com>
AuthorDate: Sat Mar 27 18:59:23 2021 +0800
[Refactor] make uint24_t,OLAPIndexFixedHeader as a POD type (#5559)
---
be/src/exprs/literal.h | 2 +-
be/src/exprs/slot_ref.h | 2 +-
be/src/olap/olap_index.h | 2 --
.../rowset/segment_v2/bloom_filter_index_writer.cpp | 10 +++++-----
be/src/olap/task/engine_clone_task.cpp | 2 +-
be/src/olap/uint24.h | 21 +++++++++------------
be/src/util/coding.h | 4 ++++
7 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/be/src/exprs/literal.h b/be/src/exprs/literal.h
index ec9616c..e0e69f5 100644
--- a/be/src/exprs/literal.h
+++ b/be/src/exprs/literal.h
@@ -25,7 +25,7 @@ namespace doris {
class TExprNode;
-class Literal : public Expr {
+class Literal final : public Expr {
public:
virtual ~Literal();
diff --git a/be/src/exprs/slot_ref.h b/be/src/exprs/slot_ref.h
index 5916a8b..2680625 100644
--- a/be/src/exprs/slot_ref.h
+++ b/be/src/exprs/slot_ref.h
@@ -27,7 +27,7 @@ namespace doris {
// We inline this here in order for Expr::get_value() to be able
// to reference SlotRef::compute_fn() directly.
// Splitting it up into separate .h files would require circular #includes.
-class SlotRef : public Expr {
+class SlotRef final : public Expr {
public:
SlotRef(const TExprNode& node);
SlotRef(const SlotDescriptor* desc);
diff --git a/be/src/olap/olap_index.h b/be/src/olap/olap_index.h
index f5f7a66..1a5e7e1 100644
--- a/be/src/olap/olap_index.h
+++ b/be/src/olap/olap_index.h
@@ -47,8 +47,6 @@ class WrapperField;
typedef uint32_t data_file_offset_t;
struct OLAPIndexFixedHeader {
- OLAPIndexFixedHeader() : data_length(0), num_rows(0) {}
-
uint32_t data_length;
uint64_t num_rows;
};
diff --git a/be/src/olap/rowset/segment_v2/bloom_filter_index_writer.cpp b/be/src/olap/rowset/segment_v2/bloom_filter_index_writer.cpp
index f0555c7..30eaf80 100644
--- a/be/src/olap/rowset/segment_v2/bloom_filter_index_writer.cpp
+++ b/be/src/olap/rowset/segment_v2/bloom_filter_index_writer.cpp
@@ -85,11 +85,11 @@ public:
const CppType* v = (const CppType*)values;
for (int i = 0; i < count; ++i) {
if (_values.find(*v) == _values.end()) {
- if (_is_slice_type()) {
+ if constexpr (_is_slice_type()) {
CppType new_value;
_typeinfo->deep_copy(&new_value, v, &_pool);
_values.insert(new_value);
- } else if (_is_int128()) {
+ } else if constexpr (_is_int128()) {
PackedInt128 new_value;
memcpy(&new_value.value, v, sizeof(PackedInt128));
_values.insert((*reinterpret_cast<CppType*>(&new_value)));
@@ -109,7 +109,7 @@ public:
RETURN_IF_ERROR(bf->init(_values.size(), _bf_options.fpp, _bf_options.strategy));
bf->set_has_null(_has_null);
for (auto& v : _values) {
- if (_is_slice_type()) {
+ if constexpr (_is_slice_type()) {
Slice* s = (Slice*)&v;
bf->add_bytes(s->data, s->size);
} else {
@@ -155,11 +155,11 @@ public:
private:
// supported slice types are: OLAP_FIELD_TYPE_CHAR|OLAP_FIELD_TYPE_VARCHAR
- bool _is_slice_type() const {
+ static constexpr bool _is_slice_type() {
return field_type == OLAP_FIELD_TYPE_VARCHAR || field_type == OLAP_FIELD_TYPE_CHAR;
}
- bool _is_int128() const { return field_type == OLAP_FIELD_TYPE_LARGEINT; }
+ static constexpr bool _is_int128() { return field_type == OLAP_FIELD_TYPE_LARGEINT; }
private:
BloomFilterOptions _bf_options;
diff --git a/be/src/olap/task/engine_clone_task.cpp b/be/src/olap/task/engine_clone_task.cpp
index dc3bda4..30d21e4 100644
--- a/be/src/olap/task/engine_clone_task.cpp
+++ b/be/src/olap/task/engine_clone_task.cpp
@@ -193,7 +193,7 @@ OLAPStatus EngineCloneTask::_do_clone() {
if (boost::filesystem::exists(local_path)) {
boost::filesystem::remove_all(local_path);
}
- } catch (boost::filesystem::filesystem_error e) {
+ } catch (boost::filesystem::filesystem_error &e) {
// Ignore the error, OLAP will delete it
LOG(WARNING) << "clone delete useless dir failed. "
<< " error: " << e.what() << " local dir: " << local_data_path.c_str()
diff --git a/be/src/olap/uint24.h b/be/src/olap/uint24.h
index d122ff6..9656469 100644
--- a/be/src/olap/uint24.h
+++ b/be/src/olap/uint24.h
@@ -18,6 +18,7 @@
#pragma once
#include <cstdint>
+#include <cstring>
#include <iostream>
#include <string>
@@ -26,13 +27,7 @@ namespace doris {
// 24bit int type, used to store date type in storage
struct uint24_t {
public:
- uint24_t() { memset(data, 0, sizeof(data)); }
-
- uint24_t(const uint24_t& value) {
- data[0] = value.data[0];
- data[1] = value.data[1];
- data[2] = value.data[2];
- }
+ uint24_t() = default;
uint24_t(const uint32_t& value) {
data[0] = static_cast<uint8_t>(value);
@@ -40,7 +35,7 @@ public:
data[2] = static_cast<uint8_t>(value >> 16);
}
- uint24_t& operator=(const uint32_t& value) {
+ uint24_t& operator=(const uint32_t value) {
data[0] = static_cast<uint8_t>(value);
data[1] = static_cast<uint8_t>(value >> 8);
data[2] = static_cast<uint8_t>(value >> 16);
@@ -54,7 +49,7 @@ public:
return *this;
}
- uint24_t& operator=(const uint64_t& value) {
+ uint24_t& operator=(const uint64_t value) {
data[0] = static_cast<uint8_t>(value);
data[1] = static_cast<uint8_t>(value >> 8);
data[2] = static_cast<uint8_t>(value >> 16);
@@ -66,7 +61,7 @@ public:
return *this;
}
- uint24_t& operator>>=(const int& bits) {
+ uint24_t& operator>>=(const int bits) {
*this = static_cast<uint>(*this) >> bits;
return *this;
}
@@ -83,14 +78,14 @@ public:
return value;
}
- uint24_t& operator=(const int& value) {
+ uint24_t& operator=(const int value) {
data[0] = static_cast<uint8_t>(value);
data[1] = static_cast<uint8_t>(value >> 8);
data[2] = static_cast<uint8_t>(value >> 16);
return *this;
}
- uint24_t& operator=(const int64_t& value) {
+ uint24_t& operator=(const int64_t value) {
data[0] = static_cast<uint8_t>(value);
data[1] = static_cast<uint8_t>(value >> 8);
data[2] = static_cast<uint8_t>(value >> 16);
@@ -147,6 +142,8 @@ private:
uint8_t data[3];
} __attribute__((packed));
+static_assert(std::is_trivial<uint24_t>::value, "uint24_t should be a POD type");
+
inline std::ostream& operator<<(std::ostream& os, const uint24_t& val) {
os << val.to_string();
return os;
diff --git a/be/src/util/coding.h b/be/src/util/coding.h
index 40ecdbb..f7b7908 100644
--- a/be/src/util/coding.h
+++ b/be/src/util/coding.h
@@ -168,6 +168,8 @@ inline const uint8_t* decode_varint32_ptr(const uint8_t* ptr, const uint8_t* lim
}
extern const uint8_t* decode_varint64_ptr(const uint8_t* p, const uint8_t* limit, uint64_t* value);
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
template <typename T>
inline void put_varint32(T* dst, uint32_t v) {
@@ -197,6 +199,8 @@ inline void put_varint64_varint32(T* dst, uint64_t v1, uint32_t v2) {
dst->append((char*)buf, static_cast<size_t>(ptr - buf));
}
+#pragma GCC diagnostic pop
+
// parse a varint32 from the start of `input` into `val`.
// on success, return true and advance `input` past the parsed value.
// on failure, return false and `input` is not modified.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org