github-actions[bot] commented on code in PR #61858:
URL: https://github.com/apache/doris/pull/61858#discussion_r3013266048
##########
be/src/storage/predicate/predicate_creator.h:
##########
@@ -17,271 +17,35 @@
#pragma once
-#include <fast_float/fast_float.h>
-
-#include <charconv>
-#include <stdexcept>
+#include <memory>
#include <string>
-#include <type_traits>
-#include "common/exception.h"
-#include "common/status.h"
#include "core/data_type/data_type.h"
-#include "core/data_type/define_primitive_type.h"
#include "core/data_type/primitive_type.h"
-#include "core/string_ref.h"
-#include "exprs/create_predicate_function.h"
-#include "exprs/function/cast/cast_parameters.h"
-#include "exprs/function/cast/cast_to_basic_number_common.h"
+#include "core/field.h"
#include "exprs/hybrid_set.h"
-#include "storage/olap_utils.h"
-#include "storage/predicate/bloom_filter_predicate.h"
#include "storage/predicate/column_predicate.h"
-#include "storage/predicate/comparison_predicate.h"
-#include "storage/predicate/in_list_predicate.h"
-#include "storage/predicate/null_predicate.h"
-#include "storage/tablet/tablet_schema.h"
-#include "util/date_func.h"
-#include "util/string_util.h"
namespace doris {
#include "common/compile_check_begin.h"
-template <PrimitiveType TYPE, PredicateType PT>
-std::shared_ptr<ColumnPredicate> create_in_list_predicate(const uint32_t cid,
- const std::string
col_name,
- const
std::shared_ptr<HybridSetBase>& set,
- bool is_opposite,
- size_t char_length =
0) {
- auto set_size = set->size();
- if (set_size == 1) {
- return InListPredicateBase<TYPE, PT, 1>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 2) {
- return InListPredicateBase<TYPE, PT, 2>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 3) {
- return InListPredicateBase<TYPE, PT, 3>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 4) {
- return InListPredicateBase<TYPE, PT, 4>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 5) {
- return InListPredicateBase<TYPE, PT, 5>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 6) {
- return InListPredicateBase<TYPE, PT, 6>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 7) {
- return InListPredicateBase<TYPE, PT, 7>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == FIXED_CONTAINER_MAX_SIZE) {
- return InListPredicateBase<TYPE, PT, 8>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else {
- return InListPredicateBase<TYPE, PT, FIXED_CONTAINER_MAX_SIZE +
1>::create_shared(
- cid, col_name, set, is_opposite, char_length);
- }
-}
+class BloomFilterFuncBase;
+class BitmapFilterFuncBase;
+// Defined in predicate_creator.cpp with explicit instantiations.
Review Comment:
Nit: This comment is inaccurate in two ways:
1. The function is defined in `predicate_creator_in_list_in.cpp` and
`predicate_creator_in_list_not_in.cpp`, not `predicate_creator.cpp`.
2. It uses explicit template **specialization** (`template <>`), not
explicit **instantiation**.
Suggested:
```cpp
// Defined in predicate_creator_in_list_in.cpp and
predicate_creator_in_list_not_in.cpp
// with explicit specializations.
```
##########
be/src/storage/predicate/predicate_creator.h:
##########
@@ -17,271 +17,35 @@
#pragma once
-#include <fast_float/fast_float.h>
-
-#include <charconv>
-#include <stdexcept>
+#include <memory>
#include <string>
-#include <type_traits>
-#include "common/exception.h"
-#include "common/status.h"
#include "core/data_type/data_type.h"
-#include "core/data_type/define_primitive_type.h"
#include "core/data_type/primitive_type.h"
-#include "core/string_ref.h"
-#include "exprs/create_predicate_function.h"
-#include "exprs/function/cast/cast_parameters.h"
-#include "exprs/function/cast/cast_to_basic_number_common.h"
+#include "core/field.h"
#include "exprs/hybrid_set.h"
-#include "storage/olap_utils.h"
-#include "storage/predicate/bloom_filter_predicate.h"
#include "storage/predicate/column_predicate.h"
-#include "storage/predicate/comparison_predicate.h"
-#include "storage/predicate/in_list_predicate.h"
-#include "storage/predicate/null_predicate.h"
-#include "storage/tablet/tablet_schema.h"
-#include "util/date_func.h"
-#include "util/string_util.h"
namespace doris {
#include "common/compile_check_begin.h"
-template <PrimitiveType TYPE, PredicateType PT>
-std::shared_ptr<ColumnPredicate> create_in_list_predicate(const uint32_t cid,
- const std::string
col_name,
- const
std::shared_ptr<HybridSetBase>& set,
- bool is_opposite,
- size_t char_length =
0) {
- auto set_size = set->size();
- if (set_size == 1) {
- return InListPredicateBase<TYPE, PT, 1>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 2) {
- return InListPredicateBase<TYPE, PT, 2>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 3) {
- return InListPredicateBase<TYPE, PT, 3>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 4) {
- return InListPredicateBase<TYPE, PT, 4>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 5) {
- return InListPredicateBase<TYPE, PT, 5>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 6) {
- return InListPredicateBase<TYPE, PT, 6>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == 7) {
- return InListPredicateBase<TYPE, PT, 7>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else if (set_size == FIXED_CONTAINER_MAX_SIZE) {
- return InListPredicateBase<TYPE, PT, 8>::create_shared(cid, col_name,
set, is_opposite,
- char_length);
- } else {
- return InListPredicateBase<TYPE, PT, FIXED_CONTAINER_MAX_SIZE +
1>::create_shared(
- cid, col_name, set, is_opposite, char_length);
- }
-}
+class BloomFilterFuncBase;
+class BitmapFilterFuncBase;
+// Defined in predicate_creator.cpp with explicit instantiations.
template <PredicateType PT>
std::shared_ptr<ColumnPredicate> create_in_list_predicate(const uint32_t cid,
const std::string
col_name,
const DataTypePtr&
data_type,
const
std::shared_ptr<HybridSetBase> set,
- bool is_opposite) {
- switch (data_type->get_primitive_type()) {
- case TYPE_TINYINT: {
- return create_in_list_predicate<TYPE_TINYINT, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_SMALLINT: {
- return create_in_list_predicate<TYPE_SMALLINT, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_INT: {
- return create_in_list_predicate<TYPE_INT, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_BIGINT: {
- return create_in_list_predicate<TYPE_BIGINT, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_LARGEINT: {
- return create_in_list_predicate<TYPE_LARGEINT, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_FLOAT: {
- return create_in_list_predicate<TYPE_FLOAT, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_DOUBLE: {
- return create_in_list_predicate<TYPE_DOUBLE, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_DECIMALV2: {
- return create_in_list_predicate<TYPE_DECIMALV2, PT>(cid, col_name,
set, is_opposite);
- }
- case TYPE_DECIMAL32: {
- return create_in_list_predicate<TYPE_DECIMAL32, PT>(cid, col_name,
set, is_opposite);
- }
- case TYPE_DECIMAL64: {
- return create_in_list_predicate<TYPE_DECIMAL64, PT>(cid, col_name,
set, is_opposite);
- }
- case TYPE_DECIMAL128I: {
- return create_in_list_predicate<TYPE_DECIMAL128I, PT>(cid, col_name,
set, is_opposite);
- }
- case TYPE_DECIMAL256: {
- return create_in_list_predicate<TYPE_DECIMAL256, PT>(cid, col_name,
set, is_opposite);
- }
- case TYPE_CHAR: {
- return create_in_list_predicate<TYPE_CHAR, PT>(
- cid, col_name, set, is_opposite,
- assert_cast<const
DataTypeString*>(remove_nullable(data_type).get())->len());
- }
- case TYPE_VARCHAR: {
- return create_in_list_predicate<TYPE_VARCHAR, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_STRING: {
- return create_in_list_predicate<TYPE_STRING, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_DATE: {
- return create_in_list_predicate<TYPE_DATE, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_DATEV2: {
- return create_in_list_predicate<TYPE_DATEV2, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_DATETIME: {
- return create_in_list_predicate<TYPE_DATETIME, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_DATETIMEV2: {
- return create_in_list_predicate<TYPE_DATETIMEV2, PT>(cid, col_name,
set, is_opposite);
- }
- case TYPE_TIMESTAMPTZ: {
- return create_in_list_predicate<TYPE_TIMESTAMPTZ, PT>(cid, col_name,
set, is_opposite);
- }
- case TYPE_BOOLEAN: {
- return create_in_list_predicate<TYPE_BOOLEAN, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_IPV4: {
- return create_in_list_predicate<TYPE_IPV4, PT>(cid, col_name, set,
is_opposite);
- }
- case TYPE_IPV6: {
- return create_in_list_predicate<TYPE_IPV6, PT>(cid, col_name, set,
is_opposite);
- }
- default:
- throw Exception(Status::InternalError("Unsupported type {} for
in_predicate",
-
type_to_string(data_type->get_primitive_type())));
- return nullptr;
- }
-}
+ bool is_opposite);
+// Defined in predicate_creator.cpp with explicit instantiations.
Review Comment:
Nit: This comment says "Defined in predicate_creator.cpp" but the function
is actually defined in `predicate_creator_comparison.cpp`.
Suggested:
```cpp
// Defined in predicate_creator_comparison.cpp with explicit instantiations.
```
##########
be/src/storage/predicate/predicate_creator_in_list_in.cpp:
##########
@@ -0,0 +1,166 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "common/exception.h"
+#include "common/status.h"
+#include "core/data_type/data_type_string.h"
+#include "storage/predicate/in_list_predicate.h"
+#include "storage/predicate/predicate_creator.h"
+
+namespace doris {
+
+template <PrimitiveType TYPE, PredicateType PT>
Review Comment:
Suggestion: `create_in_list_predicate_impl` (lines 26-59) is duplicated
identically in `predicate_creator_in_list_not_in.cpp`. Since both files have it
as a `static` function template, this works correctly but creates a maintenance
burden — if the set-size dispatching logic ever changes, both copies need to be
updated in lockstep.
Consider extracting this helper into a shared internal header (e.g.,
`predicate_creator_in_list_impl.h`) that both `.cpp` files include. This would
eliminate the duplication without affecting compilation performance since the
header would only be included by these two `.cpp` files.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
