Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix](ub) undefined behavior in FixedContainer (#39191) #39201

Merged
merged 1 commit into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions be/src/exprs/hybrid_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@

#pragma once

#include <glog/logging.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'glog/logging.h' file not found [clang-diagnostic-error]

#include <glog/logging.h>
         ^


#include <type_traits>

#include "common/exception.h"
#include "common/object_pool.h"
#include "common/status.h"
#include "exprs/runtime_filter.h"
#include "runtime/decimalv2_value.h"
#include "runtime/define_primitive_type.h"
Expand Down Expand Up @@ -60,8 +66,16 @@ class FixedContainer {
}
}

void check_size() {
if (N != _size) {
throw doris::Exception(ErrorCode::INTERNAL_ERROR,
"invalid size of FixedContainer<{}>: {}", N, _size);
}
}

// Use '|' instead of '||' has better performance by test.
ALWAYS_INLINE bool find(const T& value) const {
DCHECK_EQ(N, _size);
if constexpr (N == 0) {
return false;
}
Expand Down Expand Up @@ -144,6 +158,12 @@ class FixedContainer {
size_t _size {};
};

template <typename T>
struct IsFixedContainer : std::false_type {};

template <typename T, size_t N>
struct IsFixedContainer<FixedContainer<T, N>> : std::true_type {};

/**
* Dynamic Container uses phmap::flat_hash_set.
* @tparam T Element Type
Expand Down Expand Up @@ -354,6 +374,11 @@ class HybridSet : public HybridSetBase {
if constexpr (is_nullable) {
null_map_data = null_map->data();
}

if constexpr (IsFixedContainer<ContainerType>::value) {
_set.check_size();
}

auto* __restrict result_data = results.data();
for (size_t i = 0; i < rows; ++i) {
if constexpr (!is_nullable && !is_negative) {
Expand Down Expand Up @@ -507,6 +532,11 @@ class StringSet : public HybridSetBase {
if constexpr (is_nullable) {
null_map_data = null_map->data();
}

if constexpr (IsFixedContainer<ContainerType>::value) {
_set.check_size();
}

auto* __restrict result_data = results.data();
for (size_t i = 0; i < rows; ++i) {
const auto& string_data = col.get_data_at(i).to_string();
Expand Down Expand Up @@ -675,6 +705,11 @@ class StringValueSet : public HybridSetBase {
if constexpr (is_nullable) {
null_map_data = null_map->data();
}

if constexpr (IsFixedContainer<ContainerType>::value) {
_set.check_size();
}

auto* __restrict result_data = results.data();
for (size_t i = 0; i < rows; ++i) {
uint32_t len = offset[i] - offset[i - 1];
Expand Down
2 changes: 1 addition & 1 deletion be/src/vec/functions/in.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class FunctionIn : public IFunction {
context->get_arg_type(0)->type == PrimitiveType::TYPE_VARCHAR ||
context->get_arg_type(0)->type == PrimitiveType::TYPE_STRING) {
// the StringValue's memory is held by FunctionContext, so we can use StringValueSet here directly
state->hybrid_set.reset(create_string_value_set((size_t)(context->get_num_args() - 1)));
state->hybrid_set.reset(create_string_value_set(get_size_with_out_null(context)));
} else {
state->hybrid_set.reset(
create_set(context->get_arg_type(0)->type, get_size_with_out_null(context)));
Expand Down
9 changes: 9 additions & 0 deletions regression-test/data/nereids_syntax_p0/inpredicate.out
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,12 @@
29 Supplier#000000029 VVSymB3fbwaN ARGENTINA4 ARGENTINA AMERICA 11-773-203-7342
9 Supplier#000000009 ,gJ6K2MKveYxQT IRAN 6 IRAN MIDDLE EAST 20-338-906-3675

-- !in_predicate_11 --
15 Supplier#000000015 DF35PepL5saAK INDIA 0 INDIA ASIA 18-687-542-7601

-- !in_predicate_12 --

-- !in_predicate_13 --

-- !in_predicate_14 --

16 changes: 16 additions & 0 deletions regression-test/suites/nereids_syntax_p0/inpredicate.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,21 @@ suite("inpredicate") {
order_qt_in_predicate_10 """
SELECT * FROM supplier WHERE s_suppkey not in (15);
"""

order_qt_in_predicate_11 """
SELECT * FROM supplier WHERE s_suppkey in (15, null);
"""

order_qt_in_predicate_12 """
SELECT * FROM supplier WHERE s_suppkey not in (15, null);
"""

order_qt_in_predicate_13 """
SELECT * FROM supplier WHERE s_nation in ('PERU', 'ETHIOPIA', null);
"""

order_qt_in_predicate_14 """
SELECT * FROM supplier WHERE s_nation not in ('PERU', 'ETHIOPIA', null);
"""
}

Loading