Skip to content

Commit 477cb0c

Browse files
authored
[improvement](be) Add release-enabled Doris check macros (#63730)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Add a dedicated `common/check.h` header for Doris check macros. `DORIS_CHECK` accepts streamed context through the usual `<<` syntax while avoiding evaluation of streamed operands on successful checks. `DORIS_CHECK_EQ/NE/LT/LE/GT/GE` are intended for invariants that should remain checked in Release builds: Debug builds map them to the corresponding `DCHECK_*` macros, while Release builds evaluate each operand once, compare with the requested operator, and throw through the existing `DORIS_CHECK`-style fatal error path with a message that includes both compared expressions and their actual values. Release comparison checks also accept streamed context. `status.h` re-exports `common/check.h` to keep existing includes compatible. The JSONB function call sites that rely on these invariants are switched from `DCHECK` to the new release-enabled Doris checks. The added `DorisCheckTest` coverage exercises `check.cpp` failure handling, `check.h` value formatting helpers, binary-op result formatting, streamed messages, stream-operand laziness, comparison success, and single-evaluation behavior. ### Release note None ### Check List (For Author) - Test: Unit Test / Manual test - `build-support/clang-format.sh be/src/exprs/function/function_jsonb.cpp be/test/common/check_test.cpp` - `DORIS_HOME=$PWD ninja -C be/ut_build_ASAN src/exprs/CMakeFiles/Exprs.dir/function/function_jsonb.cpp.o test/CMakeFiles/doris_be_test.dir/common/check_test.cpp.o` - `./run-be-ut.sh --run --filter=DorisCheckTest.*` - `build-support/check-format.sh` - `git diff --cached --check` - Behavior changed: No - Does this need documentation: No
1 parent a7ad76a commit 477cb0c

5 files changed

Lines changed: 353 additions & 17 deletions

File tree

be/src/common/check.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
#include "common/check.h"
19+
20+
#include "common/exception.h"
21+
#include "common/status.h"
22+
23+
namespace doris {
24+
25+
void doris_check_fail(std::string_view message) {
26+
throw Exception(Status::FatalError("{}", message));
27+
}
28+
29+
} // namespace doris

be/src/common/check.h

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
#pragma once
19+
20+
#include <fmt/format.h>
21+
#include <glog/logging.h>
22+
23+
#include <cstddef>
24+
#include <ios>
25+
#include <ostream>
26+
#include <sstream>
27+
#include <string>
28+
#include <string_view>
29+
#include <type_traits>
30+
#include <utility>
31+
32+
namespace doris {
33+
34+
[[noreturn]] void doris_check_fail(std::string_view message);
35+
36+
namespace detail {
37+
template <typename T>
38+
concept OstreamPrintable = requires(std::ostream& os, const T& value) { os << value; };
39+
40+
class DorisCheckMessage {
41+
public:
42+
explicit DorisCheckMessage(std::string_view message) { _stream << message; }
43+
44+
template <typename T>
45+
DorisCheckMessage& operator<<(const T& value) {
46+
_stream << value;
47+
return *this;
48+
}
49+
50+
DorisCheckMessage& operator<<(std::ostream& (*func)(std::ostream&)) {
51+
func(_stream);
52+
return *this;
53+
}
54+
55+
DorisCheckMessage& operator<<(std::ios& (*func)(std::ios&)) {
56+
func(_stream);
57+
return *this;
58+
}
59+
60+
DorisCheckMessage& operator<<(std::ios_base& (*func)(std::ios_base&)) {
61+
func(_stream);
62+
return *this;
63+
}
64+
65+
[[noreturn]] void fail() { doris_check_fail(_stream.str()); }
66+
67+
private:
68+
std::ostringstream _stream;
69+
};
70+
71+
class DorisCheckMessageVoidify {
72+
public:
73+
[[noreturn]] void operator&(DorisCheckMessage& message) const { message.fail(); }
74+
[[noreturn]] void operator&(DorisCheckMessage&& message) const { message.fail(); }
75+
};
76+
77+
class DorisCheckResult {
78+
public:
79+
explicit DorisCheckResult(bool ok) : _ok(ok) {}
80+
explicit DorisCheckResult(std::string message) : _ok(false), _message(std::move(message)) {}
81+
82+
bool ok() const { return _ok; }
83+
const std::string& message() const { return _message; }
84+
85+
private:
86+
bool _ok;
87+
std::string _message;
88+
};
89+
90+
template <typename T>
91+
std::string doris_check_value_to_string(const T& value) {
92+
if constexpr (std::is_same_v<std::decay_t<T>, std::nullptr_t>) {
93+
return "nullptr";
94+
} else if constexpr (std::is_same_v<std::decay_t<T>, bool>) {
95+
return value ? "true" : "false";
96+
} else if constexpr (OstreamPrintable<T>) {
97+
std::ostringstream oss;
98+
oss << std::boolalpha << value;
99+
return oss.str();
100+
} else {
101+
return "<unprintable>";
102+
}
103+
}
104+
105+
template <typename Lhs, typename Rhs, typename Comparator>
106+
DorisCheckResult doris_check_binary_op_result(const Lhs& lhs, const Rhs& rhs,
107+
std::string_view lhs_expr, std::string_view rhs_expr,
108+
std::string_view op_expr, Comparator comparator) {
109+
if (static_cast<bool>(comparator(lhs, rhs))) {
110+
return DorisCheckResult(true);
111+
}
112+
return DorisCheckResult(fmt::format("Check failed: {} {} {} ({} vs {})", lhs_expr, op_expr,
113+
rhs_expr, doris_check_value_to_string(lhs),
114+
doris_check_value_to_string(rhs)));
115+
}
116+
} // namespace detail
117+
118+
} // namespace doris
119+
120+
// core in Debug mode, exception in Release mode.
121+
#define DORIS_CHECK(stmt) \
122+
if (bool _doris_check_ok = static_cast<bool>(stmt); _doris_check_ok) { \
123+
} else [[unlikely]] \
124+
::doris::detail::DorisCheckMessageVoidify() & \
125+
::doris::detail::DorisCheckMessage("Check failed: " #stmt)
126+
127+
// Use DORIS_CHECK_* only for invariants that must also be checked in Release builds.
128+
// Keep DCHECK_* in loops or other hot paths where Release checks would add overhead.
129+
#ifndef NDEBUG
130+
#define DORIS_CHECK_EQ(val1, val2) DCHECK_EQ(val1, val2)
131+
#define DORIS_CHECK_NE(val1, val2) DCHECK_NE(val1, val2)
132+
#define DORIS_CHECK_LT(val1, val2) DCHECK_LT(val1, val2)
133+
#define DORIS_CHECK_LE(val1, val2) DCHECK_LE(val1, val2)
134+
#define DORIS_CHECK_GT(val1, val2) DCHECK_GT(val1, val2)
135+
#define DORIS_CHECK_GE(val1, val2) DCHECK_GE(val1, val2)
136+
#else
137+
#define DORIS_CHECK_BINARY_OP(val1, val2, op, op_str) \
138+
if (auto _doris_check_result = ::doris::detail::doris_check_binary_op_result( \
139+
(val1), (val2), #val1, #val2, op_str, \
140+
[](const auto& _doris_check_lhs, const auto& _doris_check_rhs) { \
141+
return _doris_check_lhs op _doris_check_rhs; \
142+
}); \
143+
_doris_check_result.ok()) { \
144+
} else [[unlikely]] \
145+
::doris::detail::DorisCheckMessageVoidify() & \
146+
::doris::detail::DorisCheckMessage(_doris_check_result.message())
147+
148+
#define DORIS_CHECK_EQ(val1, val2) DORIS_CHECK_BINARY_OP(val1, val2, ==, "==")
149+
#define DORIS_CHECK_NE(val1, val2) DORIS_CHECK_BINARY_OP(val1, val2, !=, "!=")
150+
#define DORIS_CHECK_LT(val1, val2) DORIS_CHECK_BINARY_OP(val1, val2, <, "<")
151+
#define DORIS_CHECK_LE(val1, val2) DORIS_CHECK_BINARY_OP(val1, val2, <=, "<=")
152+
#define DORIS_CHECK_GT(val1, val2) DORIS_CHECK_BINARY_OP(val1, val2, >, ">")
153+
#define DORIS_CHECK_GE(val1, val2) DORIS_CHECK_BINARY_OP(val1, val2, >=, ">=")
154+
#endif

be/src/common/status.h

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <type_traits>
1818
#include <utility>
1919

20+
#include "common/check.h" // IWYU pragma: export
2021
#include "common/compiler_util.h" // IWYU pragma: keep
2122
#include "common/config.h"
2223
#include "common/expected.h"
@@ -769,14 +770,6 @@ using ResultError = unexpected<Status>;
769770
std::forward<_result_t>(_result_).error(); \
770771
})
771772

772-
// core in Debug mode, exception in Release mode.
773-
#define DORIS_CHECK(stmt) \
774-
do { \
775-
if (!static_cast<bool>(stmt)) [[unlikely]] { \
776-
throw Exception(Status::FatalError(fmt::format("Check failed: {}", #stmt))); \
777-
} \
778-
} while (false)
779-
780773
} // namespace doris
781774

782775
// specify formatter for Status

be/src/exprs/function/function_jsonb.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ class FunctionJsonbExtract : public IFunction {
374374

375375
Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments,
376376
uint32_t result, size_t input_rows_count) const override {
377-
DCHECK_GE(arguments.size(), 2);
377+
DORIS_CHECK_GE(arguments.size(), 2);
378378

379379
ColumnPtr jsonb_data_column;
380380
bool jsonb_data_const = false;
@@ -430,7 +430,7 @@ class FunctionJsonbExtract : public IFunction {
430430
path_null_maps, path_const, res_data, res_offsets, null_map->get_data()));
431431
} else {
432432
// not support other extract type for now (e.g. int, double, ...)
433-
DCHECK_EQ(jsonb_path_columns.size(), 1);
433+
DORIS_CHECK_EQ(jsonb_path_columns.size(), 1);
434434
const auto& rdata = jsonb_path_columns[0]->get_chars();
435435
const auto& roffsets = jsonb_path_columns[0]->get_offsets();
436436

@@ -491,8 +491,8 @@ class FunctionJsonbKeys : public IFunction {
491491

492492
Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments,
493493
uint32_t result, size_t input_rows_count) const override {
494-
DCHECK_GE(arguments.size(), 1);
495-
DCHECK(arguments.size() == 1 || arguments.size() == 2)
494+
DORIS_CHECK_GE(arguments.size(), 1);
495+
DORIS_CHECK(arguments.size() == 1 || arguments.size() == 2)
496496
<< "json_keys should have 1 or 2 arguments, but got " << arguments.size();
497497

498498
const NullMap* data_null_map = nullptr;
@@ -696,7 +696,7 @@ class FunctionJsonbExtractPath : public IFunction {
696696
path_col = assert_cast<const ColumnString*>(path_column.get());
697697
}
698698

699-
DCHECK(!(jsonb_data_const && path_const))
699+
DORIS_CHECK(!(jsonb_data_const && path_const))
700700
<< "jsonb_data_const and path_const should not be both const";
701701

702702
auto create_all_null_result = [&]() {
@@ -1367,7 +1367,7 @@ struct JsonbLengthUtil {
13671367
static Status jsonb_length_execute(FunctionContext* context, Block& block,
13681368
const ColumnNumbers& arguments, uint32_t result,
13691369
size_t input_rows_count) {
1370-
DCHECK_GE(arguments.size(), 2);
1370+
DORIS_CHECK_GE(arguments.size(), 2);
13711371
ColumnPtr jsonb_data_column;
13721372
bool jsonb_data_const = false;
13731373
// prepare jsonb data column
@@ -1482,7 +1482,7 @@ struct JsonbContainsUtil {
14821482
static Status jsonb_contains_execute(FunctionContext* context, Block& block,
14831483
const ColumnNumbers& arguments, uint32_t result,
14841484
size_t input_rows_count) {
1485-
DCHECK_GE(arguments.size(), 3);
1485+
DORIS_CHECK_GE(arguments.size(), 3);
14861486

14871487
auto jsonb_data1_column = block.get_by_position(arguments[0]).column;
14881488
auto jsonb_data2_column = block.get_by_position(arguments[1]).column;
@@ -2053,7 +2053,6 @@ class FunctionJsonbModify : public IFunction {
20532053
replace = true;
20542054
if (!build_parents_by_path(json_documents[row_idx]->getValue(),
20552055
json_path[path_index], parents)) {
2056-
DCHECK(false);
20572056
continue;
20582057
}
20592058
} else {
@@ -2725,7 +2724,7 @@ class FunctionJsonbRemove : public IFunction {
27252724

27262725
Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments,
27272726
uint32_t result, size_t input_rows_count) const override {
2728-
DCHECK_GE(arguments.size(), 2);
2727+
DORIS_CHECK_GE(arguments.size(), 2);
27292728

27302729
// Check if arguments count is valid (json_doc + at least one path)
27312730
if (arguments.size() < 2) {

0 commit comments

Comments
 (0)