This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new e5da615f9e0 [refine](column) Remove pointer access from column element
view (#64724)
e5da615f9e0 is described below
commit e5da615f9e0d7f0560a0e7e0941fe76ed7998d52
Author: Mryange <[email protected]>
AuthorDate: Thu Jun 25 09:57:55 2026 +0800
[refine](column) Remove pointer access from column element view (#64724)
### What problem does this PR solve?
ColumnElementView exposed ptr_at() only to adapt predicate set lookups
that accepted raw void pointers. For string columns this required a
mutable temporary StringRef inside the view, making the element access
API harder to reason about.
This change removes ColumnElementView::ptr_at() and the string staging
field, updates in-list predicate evaluation to use
get_element()/get_data() with HybridSetBase::find(data, size), and
replaces the predicate selector macro with a small templated helper that
accepts named lambdas.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
be/src/core/column/column_execute_util.h | 18 ++------------
be/src/storage/predicate/column_predicate.h | 32 +++++++++++++++----------
be/src/storage/predicate/comparison_predicate.h | 28 ++++++++++++----------
be/src/storage/predicate/in_list_predicate.h | 30 +++++++++++++++--------
be/src/storage/predicate/null_predicate.cpp | 8 +++----
5 files changed, 60 insertions(+), 56 deletions(-)
diff --git a/be/src/core/column/column_execute_util.h
b/be/src/core/column/column_execute_util.h
index 49d4f67684f..211191e8cae 100644
--- a/be/src/core/column/column_execute_util.h
+++ b/be/src/core/column/column_execute_util.h
@@ -36,8 +36,7 @@ namespace doris {
// Utility tools for convenient column execution
-// Per-row read view over a column. The pointer returned by ptr_at on the
-// string specialization is valid only until the next ptr_at call.
+// Per-row read view over a column.
namespace detail {
template <PrimitiveType PType>
@@ -52,7 +51,6 @@ struct NumericElementView {
ElementType get_element(size_t idx) const { return data[idx]; }
const ElementType* get_data() const { return data.data(); }
ElementType operator[](size_t idx) const { return data[idx]; }
- const ElementType* ptr_at(size_t idx) const { return data.data() + idx; }
size_t size() const { return data.size(); }
};
@@ -60,17 +58,12 @@ struct StringElementView {
using ColumnType = ColumnString;
using ElementType = StringRef;
const ColumnString& string_column;
- mutable StringRef _cell {}; // staging for ptr_at
StringElementView(const IColumn& column)
: string_column(assert_cast<const ColumnString&>(column)) {}
StringRef get_element(size_t idx) const { return
string_column.get_data_at(idx); }
StringRef operator[](size_t idx) const { return
string_column.get_data_at(idx); }
- const StringRef* ptr_at(size_t idx) const {
- _cell = string_column.get_data_at(idx);
- return &_cell;
- }
size_t size() const { return string_column.size(); }
};
@@ -80,13 +73,6 @@ template <PrimitiveType PType>
using ColumnElementView = std::conditional_t<is_string_type(PType),
detail::StringElementView,
detail::NumericElementView<PType>>;
-template <typename T>
-concept ColumnElementSubscriptable = requires(const T& v, size_t i) {
- typename T::ElementType;
- { v[i] } -> std::convertible_to<typename T::ElementType>;
- { v.size() } -> std::convertible_to<size_t>;
-};
-
// ColumnView is used to handle the nullable and const properties of a column.
// For example, a regular ColumnInt32 may appear in the following 4 cases:
// 1. ColumnInt32
@@ -155,4 +141,4 @@ struct ColumnView {
size_t size() const { return count; }
};
-} // namespace doris
\ No newline at end of file
+} // namespace doris
diff --git a/be/src/storage/predicate/column_predicate.h
b/be/src/storage/predicate/column_predicate.h
index 84d49ea4070..2bcc8359464 100644
--- a/be/src/storage/predicate/column_predicate.h
+++ b/be/src/storage/predicate/column_predicate.h
@@ -20,6 +20,7 @@
#include <memory>
#include <roaring/roaring.hh>
+#include "common/compiler_util.h"
#include "common/exception.h"
#include "core/column/column.h"
#include "core/data_type/define_primitive_type.h"
@@ -181,20 +182,25 @@ struct PredicateTypeTraits {
}
};
-#define EVALUATE_BY_SELECTOR(EVALUATE_IMPL_WITH_NULL_MAP,
EVALUATE_IMPL_WITHOUT_NULL_MAP) \
- const bool is_dense_column = pred_col.size() == size;
\
- for (uint16_t i = 0; i < size; i++) {
\
- uint16_t idx = is_dense_column ? i : sel[i];
\
- if constexpr (is_nullable) {
\
- if (EVALUATE_IMPL_WITH_NULL_MAP(idx)) {
\
- sel[new_size++] = idx;
\
- }
\
- } else {
\
- if (EVALUATE_IMPL_WITHOUT_NULL_MAP(idx)) {
\
- sel[new_size++] = idx;
\
- }
\
- }
\
+template <bool is_nullable, typename PredColumn, typename WithNullFunc,
typename WithoutNullFunc>
+inline ALWAYS_INLINE void evaluate_by_selector(const PredColumn& pred_col,
uint16_t size,
+ uint16_t* sel, uint16_t&
new_size,
+ WithNullFunc&& with_null_func,
+ WithoutNullFunc&&
without_null_func) {
+ const bool is_dense_column = pred_col.size() == size;
+ for (uint16_t i = 0; i < size; i++) {
+ uint16_t idx = is_dense_column ? i : sel[i];
+ if constexpr (is_nullable) {
+ if (with_null_func(idx)) {
+ sel[new_size++] = idx;
+ }
+ } else {
+ if (without_null_func(idx)) {
+ sel[new_size++] = idx;
+ }
+ }
}
+}
class ColumnPredicate : public std::enable_shared_from_this<ColumnPredicate> {
public:
diff --git a/be/src/storage/predicate/comparison_predicate.h
b/be/src/storage/predicate/comparison_predicate.h
index 85de8abb0ff..d420e550ff5 100644
--- a/be/src/storage/predicate/comparison_predicate.h
+++ b/be/src/storage/predicate/comparison_predicate.h
@@ -630,12 +630,14 @@ private:
}
}
uint16_t new_size = 0;
-#define EVALUATE_WITH_NULL_IMPL(IDX) \
- _opposite ^ (!null_map[IDX] && _operator(pred_col_data[IDX], dict_code))
-#define EVALUATE_WITHOUT_NULL_IMPL(IDX) _opposite ^
_operator(pred_col_data[IDX], dict_code)
- EVALUATE_BY_SELECTOR(EVALUATE_WITH_NULL_IMPL,
EVALUATE_WITHOUT_NULL_IMPL)
-#undef EVALUATE_WITH_NULL_IMPL
-#undef EVALUATE_WITHOUT_NULL_IMPL
+ auto with_null = [&](uint16_t idx) {
+ return _opposite ^ (!null_map[idx] &&
_operator(pred_col_data[idx], dict_code));
+ };
+ auto without_null = [&](uint16_t idx) {
+ return _opposite ^ _operator(pred_col_data[idx],
dict_code);
+ };
+ evaluate_by_selector<is_nullable>(pred_col, size, sel,
new_size, with_null,
+ without_null);
return new_size;
} else {
@@ -645,12 +647,14 @@ private:
} else {
uint16_t new_size = 0;
ColumnElementView<Type> pred_col {*column};
-#define EVALUATE_WITH_NULL_IMPL(IDX) \
- _opposite ^ (!null_map[IDX] && _operator(pred_col[IDX], _value))
-#define EVALUATE_WITHOUT_NULL_IMPL(IDX) _opposite ^ _operator(pred_col[IDX],
_value)
- EVALUATE_BY_SELECTOR(EVALUATE_WITH_NULL_IMPL,
EVALUATE_WITHOUT_NULL_IMPL)
-#undef EVALUATE_WITH_NULL_IMPL
-#undef EVALUATE_WITHOUT_NULL_IMPL
+ auto with_null = [&](uint16_t idx) {
+ return _opposite ^ (!null_map[idx] &&
_operator(pred_col.get_element(idx), _value));
+ };
+ auto without_null = [&](uint16_t idx) {
+ return _opposite ^ _operator(pred_col.get_element(idx),
_value);
+ };
+ evaluate_by_selector<is_nullable>(pred_col, size, sel, new_size,
with_null,
+ without_null);
return new_size;
}
}
diff --git a/be/src/storage/predicate/in_list_predicate.h
b/be/src/storage/predicate/in_list_predicate.h
index 180a1b04139..47e1a9e9cd1 100644
--- a/be/src/storage/predicate/in_list_predicate.h
+++ b/be/src/storage/predicate/in_list_predicate.h
@@ -58,6 +58,7 @@ struct std::equal_to<doris::uint24_t> {
};
namespace doris {
+
/**
* Use HybridSetType can avoid virtual function call in the loop.
* @tparam Type
@@ -536,15 +537,16 @@ private:
__builtin_unreachable();
}
} else {
- // ptr_at safe: HybridSet::find consumes the pointer synchronously.
ColumnElementView<Type> pred_col {*column};
-#define EVALUATE_WITH_NULL_IMPL(IDX) \
- is_opposite ^ (!(*null_map)[IDX] &&
_operator(_values->find(pred_col.ptr_at(IDX)), false))
-#define EVALUATE_WITHOUT_NULL_IMPL(IDX) \
- is_opposite ^ _operator(_values->find(pred_col.ptr_at(IDX)), false)
- EVALUATE_BY_SELECTOR(EVALUATE_WITH_NULL_IMPL,
EVALUATE_WITHOUT_NULL_IMPL)
-#undef EVALUATE_WITH_NULL_IMPL
-#undef EVALUATE_WITHOUT_NULL_IMPL
+ auto with_null = [&](uint16_t idx) {
+ return is_opposite ^
+ (!(*null_map)[idx] && _operator(_find_value(pred_col,
idx), false));
+ };
+ auto without_null = [&](uint16_t idx) {
+ return is_opposite ^ _operator(_find_value(pred_col, idx),
false);
+ };
+ evaluate_by_selector<is_nullable>(pred_col, size, sel, new_size,
with_null,
+ without_null);
}
return new_size;
}
@@ -592,7 +594,6 @@ private:
__builtin_unreachable();
}
} else {
- // ptr_at safe: HybridSet::find consumes the pointer synchronously.
ColumnElementView<Type> view {*column};
for (uint16_t i = 0; i < size; i++) {
if (is_and ^ flags[i]) {
@@ -607,7 +608,7 @@ private:
continue;
}
}
- bool hit = _operator(_values->find(view.ptr_at(idx)), false);
+ bool hit = _operator(_find_value(view, idx), false);
if constexpr (!is_opposite) {
if (is_and ^ hit) {
flags[i] = !is_and;
@@ -630,6 +631,15 @@ private:
}
}
+ ALWAYS_INLINE bool _find_value(const ColumnElementView<Type>& pred_col,
size_t idx) const {
+ if constexpr (is_string_type(Type)) {
+ const auto value = pred_col.get_element(idx);
+ return _values->find(value.data, value.size);
+ } else {
+ return _values->find(pred_col.get_data() + idx, sizeof(T));
+ }
+ }
+
std::shared_ptr<HybridSetBase> _values;
mutable std::map<std::pair<RowsetId, uint32_t>, std::vector<UInt8>>
_segment_id_to_value_in_dict_flags;
diff --git a/be/src/storage/predicate/null_predicate.cpp
b/be/src/storage/predicate/null_predicate.cpp
index f9c23117621..9973555109c 100644
--- a/be/src/storage/predicate/null_predicate.cpp
+++ b/be/src/storage/predicate/null_predicate.cpp
@@ -71,11 +71,9 @@ uint16_t NullPredicate::_evaluate_inner(const IColumn&
column, uint16_t* sel, ui
}
auto& pred_col = nullable->get_null_map_data();
constexpr bool is_nullable = true;
-#define EVALUATE_WITH_NULL_IMPL(IDX) pred_col[IDX] == _is_null
-#define EVALUATE_WITHOUT_NULL_IMPL(IDX) true
- EVALUATE_BY_SELECTOR(EVALUATE_WITH_NULL_IMPL,
EVALUATE_WITHOUT_NULL_IMPL)
-#undef EVALUATE_WITH_NULL_IMPL
-#undef EVALUATE_WITHOUT_NULL_IMPL
+ auto with_null = [&](uint16_t idx) { return pred_col[idx] == _is_null;
};
+ auto without_null = [](uint16_t) { return true; };
+ evaluate_by_selector<is_nullable>(pred_col, size, sel, new_size,
with_null, without_null);
return new_size;
} else {
if (_is_null) return 0;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
