You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "github-actions[bot] (via GitHub)" <gi...@apache.org> on 2023/04/10 01:45:49 UTC
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18503: [enhancement](exception) Column filter/replicate supports exception safety
github-actions[bot] commented on code in PR #18503:
URL: https://github.com/apache/doris/pull/18503#discussion_r1161376738
##########
be/src/common/status.h:
##########
@@ -206,6 +206,8 @@
E(COLUMN_STREAM_NOT_EXIST, -1716);
E(COLUMN_VALUE_NULL, -1717);
E(COLUMN_SEEK_ERROR, -1719);
+E(COLUMN_NO_MATCH_OFFSETS_SIZE - 1720);
Review Comment:
warning: too few arguments provided to function-like macro invocation [clang-diagnostic-error]
```cpp
E(COLUMN_NO_MATCH_OFFSETS_SIZE - 1720);
^
```
**be/src/common/status.h:24:** macro 'E' defined here
```cpp
#define E(name, code) static constexpr int name = code
^
```
##########
be/src/common/status.h:
##########
@@ -206,6 +206,8 @@ E(COLUMN_READ_STREAM, -1706);
E(COLUMN_STREAM_NOT_EXIST, -1716);
E(COLUMN_VALUE_NULL, -1717);
E(COLUMN_SEEK_ERROR, -1719);
+E(COLUMN_NO_MATCH_OFFSETS_SIZE - 1720);
Review Comment:
warning: C++ requires a type specifier for all declarations [clang-diagnostic-error]
```cpp
E(COLUMN_NO_MATCH_OFFSETS_SIZE - 1720);
^
```
##########
be/src/common/status.h:
##########
@@ -206,6 +206,8 @@
E(COLUMN_STREAM_NOT_EXIST, -1716);
E(COLUMN_VALUE_NULL, -1717);
E(COLUMN_SEEK_ERROR, -1719);
+E(COLUMN_NO_MATCH_OFFSETS_SIZE - 1720);
+E(COLUMN_NO_MATCH_FILTER_SIZE - 1721);
Review Comment:
warning: too few arguments provided to function-like macro invocation [clang-diagnostic-error]
```cpp
E(COLUMN_NO_MATCH_FILTER_SIZE - 1721);
^
```
**be/src/common/status.h:24:** macro 'E' defined here
```cpp
#define E(name, code) static constexpr int name = code
^
```
##########
be/src/runtime/thread_context.h:
##########
@@ -312,33 +312,22 @@ class StopCheckThreadMemTrackerLimit {
tracker->transfer_to( \
size, doris::thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker_raw())
-// Consider catching other memory errors, such as memset failure, etc.
-#define RETURN_IF_CATCH_BAD_ALLOC(stmt) \
- do { \
- doris::thread_context()->thread_mem_tracker_mgr->clear_exceed_mem_limit_msg(); \
- if (doris::enable_thread_catch_bad_alloc) { \
- try { \
- { stmt; } \
- } catch (std::bad_alloc const& e) { \
- doris::thread_context()->thread_mem_tracker()->print_log_usage( \
- doris::thread_context()->thread_mem_tracker_mgr->exceed_mem_limit_msg()); \
- return Status::MemoryLimitExceeded(fmt::format( \
- "PreCatch {}, {}", e.what(), \
- doris::thread_context()->thread_mem_tracker_mgr->exceed_mem_limit_msg())); \
- } \
- } else { \
- try { \
- doris::enable_thread_catch_bad_alloc = true; \
- Defer defer {[&]() { doris::enable_thread_catch_bad_alloc = false; }}; \
- { stmt; } \
- } catch (std::bad_alloc const& e) { \
- doris::thread_context()->thread_mem_tracker()->print_log_usage( \
- doris::thread_context()->thread_mem_tracker_mgr->exceed_mem_limit_msg()); \
- return Status::MemoryLimitExceeded(fmt::format( \
- "PreCatch {}, {}", e.what(), \
- doris::thread_context()->thread_mem_tracker_mgr->exceed_mem_limit_msg())); \
- } \
- } \
+#define RETURN_IF_CATCH_EXCEPTION(stmt) \
Review Comment:
warning: macro is not used [clang-diagnostic-unused-macros]
```cpp
#define RETURN_IF_CATCH_EXCEPTION(stmt) \
^
```
##########
be/src/vec/columns/column_array.cpp:
##########
@@ -788,8 +784,7 @@
template <typename T>
ColumnPtr ColumnArray::replicate_number(const IColumn::Offsets& replicate_offsets) const {
size_t col_size = size();
- if (col_size != replicate_offsets.size())
- LOG(FATAL) << "Size of offsets doesn't match size of column.";
+ column_match_offsets_size(col_size, replicate_offsets.size());
Review Comment:
warning: 'this' argument to member function 'column_match_offsets_size' has type 'const doris::vectorized::ColumnArray', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_offsets_size(col_size, replicate_offsets.size());
^
```
**be/src/vec/columns/column.h:693:** 'column_match_offsets_size' declared here
```cpp
void column_match_offsets_size(size_t size, size_t offsets_size) {
^
```
##########
be/src/vec/columns/column_array.cpp:
##########
@@ -910,8 +904,7 @@
ColumnPtr ColumnArray::replicate_const(const IColumn::Offsets& replicate_offsets) const {
size_t col_size = size();
- if (col_size != replicate_offsets.size())
- LOG(FATAL) << "Size of offsets doesn't match size of column.";
+ column_match_offsets_size(col_size, replicate_offsets.size());
Review Comment:
warning: 'this' argument to member function 'column_match_offsets_size' has type 'const doris::vectorized::ColumnArray', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_offsets_size(col_size, replicate_offsets.size());
^
```
**be/src/vec/columns/column.h:693:** 'column_match_offsets_size' declared here
```cpp
void column_match_offsets_size(size_t size, size_t offsets_size) {
^
```
##########
be/src/vec/columns/column_complex.h:
##########
@@ -313,9 +313,7 @@ template <typename T>
ColumnPtr ColumnComplexType<T>::filter(const IColumn::Filter& filt,
ssize_t result_size_hint) const {
size_t size = data.size();
- if (size != filt.size()) {
- LOG(FATAL) << "Size of filter doesn't match size of column.";
- }
+ column_match_filter_size(size, filt.size());
Review Comment:
warning: use of undeclared identifier 'column_match_filter_size' [clang-diagnostic-error]
```cpp
column_match_filter_size(size, filt.size());
^
```
##########
be/src/vec/columns/column_const.cpp:
##########
@@ -55,30 +55,21 @@ ColumnPtr ColumnConst::remove_low_cardinality() const {
}
ColumnPtr ColumnConst::filter(const Filter& filt, ssize_t /*result_size_hint*/) const {
- if (s != filt.size()) {
- LOG(FATAL) << fmt::format("Size of filter ({}) doesn't match size of column ({})",
- filt.size(), s);
- }
+ column_match_filter_size(s, filt.size());
Review Comment:
warning: 'this' argument to member function 'column_match_filter_size' has type 'const doris::vectorized::ColumnConst', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_filter_size(s, filt.size());
^
```
**be/src/vec/columns/column.h:702:** 'column_match_filter_size' declared here
```cpp
void column_match_filter_size(size_t size, size_t filter_size) {
^
```
##########
be/src/vec/columns/column_decimal.cpp:
##########
@@ -382,9 +378,7 @@
template <typename T>
ColumnPtr ColumnDecimal<T>::replicate(const IColumn::Offsets& offsets) const {
size_t size = data.size();
- if (size != offsets.size()) {
- LOG(FATAL) << "Size of offsets doesn't match size of column.";
- }
+ column_match_offsets_size(size, offsets.size());
Review Comment:
warning: use of undeclared identifier 'column_match_offsets_size' [clang-diagnostic-error]
```cpp
column_match_offsets_size(size, offsets.size());
^
```
##########
be/src/vec/columns/column_string.cpp:
##########
@@ -377,9 +377,7 @@ void ColumnString::get_permutation(bool reverse, size_t limit, int /*nan_directi
ColumnPtr ColumnString::replicate(const Offsets& replicate_offsets) const {
size_t col_size = size();
- if (col_size != replicate_offsets.size()) {
- LOG(FATAL) << "Size of offsets doesn't match size of column.";
- }
+ column_match_offsets_size(col_size, replicate_offsets.size());
Review Comment:
warning: 'this' argument to member function 'column_match_offsets_size' has type 'const doris::vectorized::ColumnString', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_offsets_size(col_size, replicate_offsets.size());
^
```
**be/src/vec/columns/column.h:693:** 'column_match_offsets_size' declared here
```cpp
void column_match_offsets_size(size_t size, size_t offsets_size) {
^
```
##########
be/src/common/status.h:
##########
@@ -206,6 +206,8 @@
E(COLUMN_STREAM_NOT_EXIST, -1716);
E(COLUMN_VALUE_NULL, -1717);
E(COLUMN_SEEK_ERROR, -1719);
+E(COLUMN_NO_MATCH_OFFSETS_SIZE - 1720);
+E(COLUMN_NO_MATCH_FILTER_SIZE - 1721);
Review Comment:
warning: C++ requires a type specifier for all declarations [clang-diagnostic-error]
```cpp
E(COLUMN_NO_MATCH_FILTER_SIZE - 1721);
^
```
##########
be/src/vec/columns/column.h:
##########
@@ -691,6 +691,24 @@ class IColumn : public COW<IColumn> {
}
}
+ void column_match_offsets_size(size_t size, size_t offsets_size) {
+ if (size != offsets_size) {
+ throw doris::Exception(
+ ErrorCode::COLUMN_NO_MATCH_OFFSETS_SIZE,
Review Comment:
warning: no member named 'COLUMN_NO_MATCH_OFFSETS_SIZE' in namespace 'doris::ErrorCode' [clang-diagnostic-error]
```cpp
ErrorCode::COLUMN_NO_MATCH_OFFSETS_SIZE,
^
```
##########
be/src/vec/columns/column_array.cpp:
##########
@@ -440,7 +440,7 @@ size_t ColumnArray::filter_number(const Filter& filter) {
ColumnPtr ColumnArray::filter_string(const Filter& filt, ssize_t result_size_hint) const {
size_t col_size = get_offsets().size();
- if (col_size != filt.size()) LOG(FATAL) << "Size of filter doesn't match size of column.";
+ column_match_filter_size(col_size, filt.size());
Review Comment:
warning: 'this' argument to member function 'column_match_filter_size' has type 'const doris::vectorized::ColumnArray', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_filter_size(col_size, filt.size());
^
```
**be/src/vec/columns/column.h:702:** 'column_match_filter_size' declared here
```cpp
void column_match_filter_size(size_t size, size_t filter_size) {
^
```
##########
be/src/vec/columns/column.h:
##########
@@ -691,6 +691,24 @@
}
}
+ void column_match_offsets_size(size_t size, size_t offsets_size) {
+ if (size != offsets_size) {
+ throw doris::Exception(
+ ErrorCode::COLUMN_NO_MATCH_OFFSETS_SIZE,
+ "Size of offsets doesn't match size of column: size={}, offsets.size={}", size,
+ offsets_size);
+ }
+ }
+
+ void column_match_filter_size(size_t size, size_t filter_size) {
+ if (size != filter_size) {
+ throw doris::Exception(
+ ErrorCode::COLUMN_NO_MATCH_FILTER_SIZE,
Review Comment:
warning: no member named 'COLUMN_NO_MATCH_FILTER_SIZE' in namespace 'doris::ErrorCode' [clang-diagnostic-error]
```cpp
ErrorCode::COLUMN_NO_MATCH_FILTER_SIZE,
^
```
##########
be/src/vec/columns/column_array.cpp:
##########
@@ -567,7 +565,7 @@
ColumnPtr ColumnArray::filter_generic(const Filter& filt, ssize_t result_size_hint) const {
size_t size = get_offsets().size();
- if (size != filt.size()) LOG(FATAL) << "Size of filter doesn't match size of column.";
+ column_match_filter_size(size, filt.size());
Review Comment:
warning: 'this' argument to member function 'column_match_filter_size' has type 'const doris::vectorized::ColumnArray', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_filter_size(size, filt.size());
^
```
**be/src/vec/columns/column.h:702:** 'column_match_filter_size' declared here
```cpp
void column_match_filter_size(size_t size, size_t filter_size) {
^
```
##########
be/src/vec/columns/column_array.cpp:
##########
@@ -944,8 +937,7 @@
ColumnPtr ColumnArray::replicate_generic(const IColumn::Offsets& replicate_offsets) const {
size_t col_size = size();
- if (col_size != replicate_offsets.size())
- LOG(FATAL) << "Size of offsets doesn't match size of column.";
+ column_match_offsets_size(col_size, replicate_offsets.size());
Review Comment:
warning: 'this' argument to member function 'column_match_offsets_size' has type 'const doris::vectorized::ColumnArray', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_offsets_size(col_size, replicate_offsets.size());
^
```
**be/src/vec/columns/column.h:693:** 'column_match_offsets_size' declared here
```cpp
void column_match_offsets_size(size_t size, size_t offsets_size) {
^
```
##########
be/src/vec/columns/column_array.cpp:
##########
@@ -836,8 +831,7 @@
ColumnPtr ColumnArray::replicate_string(const IColumn::Offsets& replicate_offsets) const {
size_t col_size = size();
- if (col_size != replicate_offsets.size())
- LOG(FATAL) << "Size of offsets doesn't match size of column.";
+ column_match_offsets_size(col_size, replicate_offsets.size());
Review Comment:
warning: 'this' argument to member function 'column_match_offsets_size' has type 'const doris::vectorized::ColumnArray', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_offsets_size(col_size, replicate_offsets.size());
^
```
**be/src/vec/columns/column.h:693:** 'column_match_offsets_size' declared here
```cpp
void column_match_offsets_size(size_t size, size_t offsets_size) {
^
```
##########
be/src/vec/columns/column_complex.h:
##########
@@ -340,9 +338,7 @@
template <typename T>
size_t ColumnComplexType<T>::filter(const IColumn::Filter& filter) {
size_t size = data.size();
- if (size != filter.size()) {
- LOG(FATAL) << "Size of filter doesn't match size of column.";
- }
+ column_match_filter_size(size, filter.size());
Review Comment:
warning: use of undeclared identifier 'column_match_filter_size' [clang-diagnostic-error]
```cpp
column_match_filter_size(size, filter.size());
^
```
##########
be/src/vec/columns/column_decimal.cpp:
##########
@@ -276,9 +276,7 @@ void ColumnDecimal<T>::insert_range_from(const IColumn& src, size_t start, size_
template <typename T>
ColumnPtr ColumnDecimal<T>::filter(const IColumn::Filter& filt, ssize_t result_size_hint) const {
size_t size = data.size();
- if (size != filt.size()) {
- LOG(FATAL) << "Size of filter doesn't match size of column.";
- }
+ column_match_filter_size(size, filt.size());
Review Comment:
warning: use of undeclared identifier 'column_match_filter_size' [clang-diagnostic-error]
```cpp
column_match_filter_size(size, filt.size());
^
```
##########
be/src/vec/columns/column_complex.h:
##########
@@ -391,9 +387,7 @@
template <typename T>
ColumnPtr ColumnComplexType<T>::replicate(const IColumn::Offsets& offsets) const {
size_t size = data.size();
- if (size != offsets.size()) {
- LOG(FATAL) << "Size of offsets doesn't match size of column.";
- }
+ column_match_offsets_size(size, offsets.size());
Review Comment:
warning: use of undeclared identifier 'column_match_offsets_size' [clang-diagnostic-error]
```cpp
column_match_offsets_size(size, offsets.size());
^
```
##########
be/src/vec/columns/column_const.cpp:
##########
@@ -55,30 +55,21 @@
}
ColumnPtr ColumnConst::filter(const Filter& filt, ssize_t /*result_size_hint*/) const {
- if (s != filt.size()) {
- LOG(FATAL) << fmt::format("Size of filter ({}) doesn't match size of column ({})",
- filt.size(), s);
- }
+ column_match_filter_size(s, filt.size());
return ColumnConst::create(data, count_bytes_in_filter(filt));
}
size_t ColumnConst::filter(const Filter& filter) {
- if (s != filter.size()) {
- LOG(FATAL) << fmt::format("Size of filter ({}) doesn't match size of column ({})",
- filter.size(), s);
- }
+ column_match_filter_size(s, filter.size());
const auto result_size = count_bytes_in_filter(filter);
resize(result_size);
return result_size;
}
ColumnPtr ColumnConst::replicate(const Offsets& offsets) const {
- if (s != offsets.size()) {
- LOG(FATAL) << fmt::format("Size of offsets ({}) doesn't match size of column ({})",
- offsets.size(), s);
- }
+ column_match_offsets_size(s, offsets.size());
Review Comment:
warning: 'this' argument to member function 'column_match_offsets_size' has type 'const doris::vectorized::ColumnConst', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_offsets_size(s, offsets.size());
^
```
**be/src/vec/columns/column.h:693:** 'column_match_offsets_size' declared here
```cpp
void column_match_offsets_size(size_t size, size_t offsets_size) {
^
```
##########
be/src/vec/columns/column_vector.cpp:
##########
@@ -387,10 +387,7 @@ void ColumnVector<T>::insert_indices_from(const IColumn& src, const int* indices
template <typename T>
ColumnPtr ColumnVector<T>::filter(const IColumn::Filter& filt, ssize_t result_size_hint) const {
size_t size = data.size();
- if (size != filt.size()) {
- LOG(FATAL) << "Size of filter doesn't match size of column. data size: " << size
- << ", filter size: " << filt.size() << get_stack_trace();
- }
+ column_match_filter_size(size, filt.size());
Review Comment:
warning: use of undeclared identifier 'column_match_filter_size' [clang-diagnostic-error]
```cpp
column_match_filter_size(size, filt.size());
^
```
##########
be/src/vec/columns/column_dummy.h:
##########
@@ -108,9 +108,7 @@ class IColumnDummy : public IColumn {
}
ColumnPtr replicate(const Offsets& offsets) const override {
- if (s != offsets.size()) {
- LOG(FATAL) << "Size of offsets doesn't match size of column.";
- }
+ column_match_offsets_size(s, offsets.size());
Review Comment:
warning: 'this' argument to member function 'column_match_offsets_size' has type 'const doris::vectorized::IColumnDummy', but function is not marked const [clang-diagnostic-error]
```cpp
column_match_offsets_size(s, offsets.size());
^
```
**be/src/vec/columns/column.h:693:** 'column_match_offsets_size' declared here
```cpp
void column_match_offsets_size(size_t size, size_t offsets_size) {
^
```
##########
be/src/vec/columns/column_vector.cpp:
##########
@@ -444,10 +441,7 @@
template <typename T>
size_t ColumnVector<T>::filter(const IColumn::Filter& filter) {
size_t size = data.size();
- if (size != filter.size()) {
- LOG(FATAL) << "Size of filter doesn't match size of column. data size: " << size
- << ", filter size: " << filter.size() << get_stack_trace();
- }
+ column_match_filter_size(size, filter.size());
Review Comment:
warning: use of undeclared identifier 'column_match_filter_size' [clang-diagnostic-error]
```cpp
column_match_filter_size(size, filter.size());
^
```
##########
be/src/vec/columns/column_decimal.cpp:
##########
@@ -327,9 +325,7 @@
template <typename T>
size_t ColumnDecimal<T>::filter(const IColumn::Filter& filter) {
size_t size = data.size();
- if (size != filter.size()) {
- LOG(FATAL) << "Size of filter doesn't match size of column.";
- }
+ column_match_filter_size(size, filter.size());
Review Comment:
warning: use of undeclared identifier 'column_match_filter_size' [clang-diagnostic-error]
```cpp
column_match_filter_size(size, filter.size());
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org