Skip to content

Commit a51f0fc

Browse files
[BugFix] frame_start/frame_end is modified unexpectedly when the window contains lead/lag and other agg (backport #58437) (#58460)
Signed-off-by: satanson <[email protected]> Co-authored-by: satanson <[email protected]>
1 parent b5be845 commit a51f0fc

File tree

3 files changed

+67
-4
lines changed

3 files changed

+67
-4
lines changed

be/src/exec/analytor.cpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -956,24 +956,26 @@ void Analytor::_update_window_batch(int64_t partition_start, int64_t partition_e
956956
data_columns[j] = _agg_intput_columns[i][j].get();
957957
}
958958

959+
auto current_frame_start = frame_start;
960+
auto current_frame_end = frame_end;
959961
// For lead/lag function, it uses the relationship between the frame_start and frame_end to determine
960962
// whether NULL value should be generated, so the frame should not be normalized.
961963
if (!_is_lead_lag_functions[i]) {
962-
frame_start = std::max<int64_t>(frame_start, _partition.start);
964+
current_frame_start = std::max<int64_t>(current_frame_start, _partition.start);
963965
// For half unounded window, we have not found the partition end, _found_partition_end.second refers to the next position.
964966
// And for others, _found_partition_end.second is identical to _partition.end, so we can always use _found_partition_end
965967
// instead of _partition.end to refer to the current right boundary.
966-
frame_end = std::min<int64_t>(frame_end, _partition.end);
968+
current_frame_end = std::min<int64_t>(current_frame_end, _partition.end);
967969
}
968970
if (_is_merge_funcs) {
969-
for (size_t j = frame_start; j < frame_end; j++) {
971+
for (size_t j = current_frame_start; j < current_frame_end; j++) {
970972
_agg_functions[i]->merge(_agg_fn_ctxs[i], data_columns[0],
971973
_managed_fn_states[0]->mutable_data() + _agg_states_offsets[i], j);
972974
}
973975
} else {
974976
_agg_functions[i]->update_batch_single_state_with_frame(
975977
_agg_fn_ctxs[i], _managed_fn_states[0]->mutable_data() + _agg_states_offsets[i], data_columns,
976-
partition_start, partition_end, frame_start, frame_end);
978+
partition_start, partition_end, current_frame_start, current_frame_end);
977979
}
978980
}
979981
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
-- name: test_window_contains_lead_lag_and_aggs
2+
create table t0 (c0 int, c1 int,c2 int) properties("replication_num"="1");
3+
-- result:
4+
-- !result
5+
insert into t0 values (1,2,3),(1,3,5),(1,4,3),(2,1,1),(2,3,4),(2,5,6);
6+
-- result:
7+
-- !result
8+
create table result_tab(r bigint) properties("replication_num"="1");
9+
-- result:
10+
-- !result
11+
insert into result_tab
12+
with cte as(
13+
SELECT c0,c1,c2,
14+
sum(c2) OVER(PARTITION BY c0 ORDER BY c1 ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) c3,
15+
max(c2) OVER(PARTITION BY c0 ORDER BY c1 ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) c4,
16+
lead(c2,1,0) OVER(PARTITION BY c0 ORDER BY c1) c5
17+
FROM t0)
18+
select sum(murmur_hash3_32(c0,c1,c2,c3,c4,c5)) from cte;
19+
-- result:
20+
-- !result
21+
insert into result_tab
22+
with cte as(
23+
SELECT c0,c1,c2,
24+
lead(c2,1,0) OVER(PARTITION BY c0 ORDER BY c1) c3,
25+
sum(c2) OVER(PARTITION BY c0 ORDER BY c1 ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) c4,
26+
max(c2) OVER(PARTITION BY c0 ORDER BY c1 ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) c5
27+
FROM t0)
28+
select sum(murmur_hash3_32(c0,c1,c2,c4,c5,c3)) from cte;
29+
-- result:
30+
-- !result
31+
select assert_true(count(distinct r)=1), assert_true(count(1)=2) from result_tab;
32+
-- result:
33+
1 1
34+
-- !result
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
-- name: test_window_contains_lead_lag_and_aggs
2+
3+
create table t0 (c0 int, c1 int,c2 int) properties("replication_num"="1");
4+
5+
insert into t0 values (1,2,3),(1,3,5),(1,4,3),(2,1,1),(2,3,4),(2,5,6);
6+
7+
create table result_tab(r bigint) properties("replication_num"="1");
8+
9+
insert into result_tab
10+
with cte as(
11+
SELECT c0,c1,c2,
12+
sum(c2) OVER(PARTITION BY c0 ORDER BY c1 ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) c3,
13+
max(c2) OVER(PARTITION BY c0 ORDER BY c1 ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) c4,
14+
lead(c2,1,0) OVER(PARTITION BY c0 ORDER BY c1) c5
15+
FROM t0)
16+
select sum(murmur_hash3_32(c0,c1,c2,c3,c4,c5)) from cte;
17+
18+
insert into result_tab
19+
with cte as(
20+
SELECT c0,c1,c2,
21+
lead(c2,1,0) OVER(PARTITION BY c0 ORDER BY c1) c3,
22+
sum(c2) OVER(PARTITION BY c0 ORDER BY c1 ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) c4,
23+
max(c2) OVER(PARTITION BY c0 ORDER BY c1 ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) c5
24+
FROM t0)
25+
select sum(murmur_hash3_32(c0,c1,c2,c4,c5,c3)) from cte;
26+
27+
select assert_true(count(distinct r)=1), assert_true(count(1)=2) from result_tab;

0 commit comments

Comments
 (0)