Skip to content

Commit 4910a5a

Browse files
authored
[fix](window_funnel) fix wrong result of window_funnel (apache#38954)
## Proposed changes Issue Number: close #xxx Current logic of `window_funnel` is wrong, it cannot express the semantic of the function. This PR re-implement the logic.
1 parent 333bebd commit 4910a5a

File tree

10 files changed

+948
-57
lines changed

10 files changed

+948
-57
lines changed

be/src/agent/be_exec_version_manager.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class BeExecVersionManager {
8282
* d. change some agg function nullable property: PR #37215
8383
* e. change variant serde to fix PR #38413
8484
*/
85-
constexpr inline int BeExecVersionManager::max_be_exec_version = 6;
85+
constexpr inline int BeExecVersionManager::max_be_exec_version = 7;
8686
constexpr inline int BeExecVersionManager::min_be_exec_version = 0;
8787

8888
/// functional

be/src/util/simd/bits.h

+16
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,18 @@ static size_t find_byte(const std::vector<T>& vec, size_t start, T byte) {
136136
return (T*)p - vec.data();
137137
}
138138

139+
template <class T>
140+
static size_t find_byte(const T* data, size_t start, size_t end, T byte) {
141+
if (start >= end) {
142+
return start;
143+
}
144+
const void* p = std::memchr((const void*)(data + start), byte, end - start);
145+
if (p == nullptr) {
146+
return end;
147+
}
148+
return (T*)p - data;
149+
}
150+
139151
template <typename T>
140152
bool contain_byte(const T* __restrict data, const size_t length, const signed char byte) {
141153
return nullptr != std::memchr(reinterpret_cast<const void*>(data), byte, length);
@@ -145,6 +157,10 @@ inline size_t find_one(const std::vector<uint8_t>& vec, size_t start) {
145157
return find_byte<uint8_t>(vec, start, 1);
146158
}
147159

160+
inline size_t find_one(const uint8_t* data, size_t start, size_t end) {
161+
return find_byte<uint8_t>(data, start, end, 1);
162+
}
163+
148164
inline size_t find_zero(const std::vector<uint8_t>& vec, size_t start) {
149165
return find_byte<uint8_t>(vec, start, 0);
150166
}

be/src/vec/aggregate_functions/aggregate_function_simple_factory.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ void register_aggregate_function_group_concat(AggregateFunctionSimpleFactory& fa
5555
void register_aggregate_function_percentile(AggregateFunctionSimpleFactory& factory);
5656
void register_aggregate_function_percentile_old(AggregateFunctionSimpleFactory& factory);
5757
void register_aggregate_function_window_funnel(AggregateFunctionSimpleFactory& factory);
58+
void register_aggregate_function_window_funnel_old(AggregateFunctionSimpleFactory& factory);
5859
void register_aggregate_function_retention(AggregateFunctionSimpleFactory& factory);
5960
void register_aggregate_function_percentile_approx(AggregateFunctionSimpleFactory& factory);
6061
void register_aggregate_function_orthogonal_bitmap(AggregateFunctionSimpleFactory& factory);
@@ -98,6 +99,7 @@ AggregateFunctionSimpleFactory& AggregateFunctionSimpleFactory::instance() {
9899
register_aggregate_function_percentile_old(instance);
99100
register_aggregate_function_percentile_approx(instance);
100101
register_aggregate_function_window_funnel(instance);
102+
register_aggregate_function_window_funnel_old(instance);
101103
register_aggregate_function_retention(instance);
102104
register_aggregate_function_orthogonal_bitmap(instance);
103105
register_aggregate_function_collect_list(instance);

be/src/vec/aggregate_functions/aggregate_function_window_funnel.cpp

+32-3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "common/logging.h"
2525
#include "vec/aggregate_functions/aggregate_function_simple_factory.h"
2626
#include "vec/aggregate_functions/helpers.h"
27+
#include "vec/core/types.h"
2728
#include "vec/data_types/data_type.h"
2829
#include "vec/data_types/data_type_nullable.h"
2930

@@ -38,11 +39,33 @@ AggregateFunctionPtr create_aggregate_function_window_funnel(const std::string&
3839
}
3940
if (WhichDataType(remove_nullable(argument_types[2])).is_date_time_v2()) {
4041
return creator_without_type::create<
41-
AggregateFunctionWindowFunnel<DateV2Value<DateTimeV2ValueType>, UInt64>>(
42-
argument_types, result_is_nullable);
42+
AggregateFunctionWindowFunnel<TypeIndex::DateTimeV2, UInt64>>(argument_types,
43+
result_is_nullable);
4344
} else if (WhichDataType(remove_nullable(argument_types[2])).is_date_time()) {
44-
return creator_without_type::create<AggregateFunctionWindowFunnel<VecDateTimeValue, Int64>>(
45+
return creator_without_type::create<
46+
AggregateFunctionWindowFunnel<TypeIndex::DateTime, Int64>>(argument_types,
47+
result_is_nullable);
48+
} else {
49+
LOG(WARNING) << "Only support DateTime type as window argument!";
50+
return nullptr;
51+
}
52+
}
53+
54+
AggregateFunctionPtr create_aggregate_function_window_funnel_old(const std::string& name,
55+
const DataTypes& argument_types,
56+
const bool result_is_nullable) {
57+
if (argument_types.size() < 3) {
58+
LOG(WARNING) << "window_funnel's argument less than 3.";
59+
return nullptr;
60+
}
61+
if (WhichDataType(remove_nullable(argument_types[2])).is_date_time_v2()) {
62+
return creator_without_type::create<
63+
AggregateFunctionWindowFunnelOld<DateV2Value<DateTimeV2ValueType>, UInt64>>(
4564
argument_types, result_is_nullable);
65+
} else if (WhichDataType(remove_nullable(argument_types[2])).is_date_time()) {
66+
return creator_without_type::create<
67+
AggregateFunctionWindowFunnelOld<VecDateTimeValue, Int64>>(argument_types,
68+
result_is_nullable);
4669
} else {
4770
LOG(WARNING) << "Only support DateTime type as window argument!";
4871
return nullptr;
@@ -52,4 +75,10 @@ AggregateFunctionPtr create_aggregate_function_window_funnel(const std::string&
5275
void register_aggregate_function_window_funnel(AggregateFunctionSimpleFactory& factory) {
5376
factory.register_function_both("window_funnel", create_aggregate_function_window_funnel);
5477
}
78+
void register_aggregate_function_window_funnel_old(AggregateFunctionSimpleFactory& factory) {
79+
factory.register_alternative_function("window_funnel",
80+
create_aggregate_function_window_funnel_old, true);
81+
factory.register_alternative_function("window_funnel",
82+
create_aggregate_function_window_funnel_old, false);
83+
}
5584
} // namespace doris::vectorized

0 commit comments

Comments
 (0)