Skip to content

Commit 182b744

Browse files
authored
fix(query): fix and check total_buffer_len and total_bytes_len (#16854)
* chore(ci): enable debug_assertions in release build ci * fix(query): fix and check total_buffer_len and total_bytes_len * fix(query): fix and check total_buffer_len and total_bytes_len * reset ci build * update * update * update
1 parent 100bf47 commit 182b744

File tree

10 files changed

+56
-12
lines changed

10 files changed

+56
-12
lines changed

src/common/arrow/src/arrow/array/binview/mod.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,18 @@ impl<T: ViewType + ?Sized> BinaryViewArrayGeneric<T> {
166166
total_bytes_len: usize,
167167
total_buffer_len: usize,
168168
) -> Self {
169+
#[cfg(debug_assertions)]
170+
{
171+
if total_bytes_len != UNKNOWN_LEN as usize {
172+
let total = views.iter().map(|v| v.length as usize).sum::<usize>();
173+
assert_eq!(total, total_bytes_len);
174+
}
175+
176+
if total_buffer_len != UNKNOWN_LEN as usize {
177+
let total = buffers.iter().map(|v| v.len()).sum::<usize>();
178+
assert_eq!(total, total_buffer_len);
179+
}
180+
}
169181
// # Safety
170182
// The caller must ensure
171183
// - the data is valid utf8 (if required)

src/common/arrow/src/arrow/array/binview/mutable.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,12 +155,11 @@ impl<T: ViewType + ?Sized> MutableBinaryViewArray<T> {
155155
#[inline]
156156
pub(crate) unsafe fn push_view_unchecked(&mut self, v: View, buffers: &[Buffer<u8>]) {
157157
let len = v.length;
158-
self.total_bytes_len += len as usize;
159158
if len <= 12 {
159+
self.total_bytes_len += len as usize;
160160
debug_assert!(self.views.capacity() > self.views.len());
161161
self.views.push(v)
162162
} else {
163-
self.total_buffer_len += len as usize;
164163
let data = buffers.get_unchecked(v.buffer_idx as usize);
165164
let offset = v.offset as usize;
166165
let bytes = data.get_unchecked(offset..offset + len as usize);
@@ -263,12 +262,19 @@ impl<T: ViewType + ?Sized> MutableBinaryViewArray<T> {
263262
// Push and pop to get the properly encoded value.
264263
// For long string this leads to a dictionary encoding,
265264
// as we push the string only once in the buffers
265+
266+
let old_bytes_len = self.total_bytes_len;
267+
266268
let view_value = value
267269
.map(|v| {
268270
self.push_value_ignore_validity(v);
269271
self.views.pop().unwrap()
270272
})
271273
.unwrap_or_default();
274+
275+
self.total_bytes_len +=
276+
(self.total_bytes_len - old_bytes_len) * additional.saturating_sub(1);
277+
272278
self.views
273279
.extend(std::iter::repeat(view_value).take(additional));
274280
}

src/common/arrow/src/arrow/array/growable/binview.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ impl<'a, T: ViewType + ?Sized> GrowableBinaryViewArray<'a, T> {
7474
capacity: usize,
7575
) -> Self {
7676
let data_type = arrays[0].data_type().clone();
77-
7877
// if any of the arrays has nulls, insertions from any array requires setting bits
7978
// as there is at least one array with nulls.
8079
if !use_validity & arrays.iter().any(|array| array.null_count() > 0) {
@@ -91,11 +90,8 @@ impl<'a, T: ViewType + ?Sized> GrowableBinaryViewArray<'a, T> {
9190
.map(|buf| BufferKey { inner: buf })
9291
})
9392
.collect::<ArrowIndexSet<_>>();
94-
let total_buffer_len = arrays
95-
.iter()
96-
.map(|arr| arr.data_buffers().len())
97-
.sum::<usize>();
9893

94+
let total_buffer_len = buffers.iter().map(|v| v.inner.len()).sum();
9995
Self {
10096
arrays,
10197
data_type,

src/common/arrow/tests/it/arrow/array/binview/mutable_values.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,17 @@ fn extend_from_iter() {
2929
.as_box()
3030
)
3131
}
32+
33+
#[test]
34+
fn extend_from_repeats() {
35+
let mut b = MutableBinaryViewArray::<str>::new();
36+
b.extend_constant(4, Some("databend"));
37+
38+
let a = b.clone();
39+
b.extend_trusted_len_values(a.values_iter());
40+
41+
assert_eq!(
42+
b.as_box(),
43+
MutableBinaryViewArray::<str>::from_values_iter(vec!["databend"; 8].into_iter()).as_box()
44+
)
45+
}

src/query/expression/src/kernels/filter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ impl<'a> FilterVisitor<'a> {
358358
new_views,
359359
values.data.data_buffers().clone(),
360360
None,
361-
Some(values.data.total_buffer_len()),
361+
None,
362362
)
363363
};
364364
StringColumn::new(new_col)

src/query/expression/src/kernels/take.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ where I: databend_common_arrow::arrow::types::Index
291291
new_views,
292292
col.data.data_buffers().clone(),
293293
None,
294-
Some(col.data.total_buffer_len()),
294+
None,
295295
)
296296
};
297297
StringColumn::new(new_col)

src/query/expression/src/kernels/take_compact.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ impl<'a> TakeCompactVisitor<'a> {
238238
new_views,
239239
col.data.data_buffers().clone(),
240240
None,
241-
Some(col.data.total_buffer_len()),
241+
None,
242242
)
243243
};
244244
StringColumn::new(new_col)

src/query/expression/src/kernels/take_ranges.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ impl<'a> TakeRangeVisitor<'a> {
239239
new_views,
240240
col.data.data_buffers().clone(),
241241
None,
242-
Some(col.data.total_buffer_len()),
242+
None,
243243
)
244244
};
245245
StringColumn::new(new_col)

tests/sqllogictests/suites/base/01_system/01_0001_system_tables.test

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ select name, database, owner from system.tables where database='c' and name ='t1
7171
----
7272
t100 c account_admin
7373

74+
statement ok
75+
select * from system.malloc_stats_totals;
76+
77+
statement ok
78+
select * from system.malloc_stats;
79+
7480
statement ok
7581
drop database if exists a;
7682

tests/sqllogictests/suites/query/join/left_outer.test

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,8 +298,18 @@ SELECT * FROM t1 LEFT JOIN t2 ON t1.a = t2.a;
298298
4 5 NULL NULL
299299
5 6 NULL NULL
300300

301+
302+
statement ok
303+
create or replace table t1 (a string) as select number from numbers(100);
304+
305+
statement ok
306+
create or replace table t2 (a string) as select number from numbers(100);
307+
308+
## just check it works or not
301309
statement ok
302-
set max_block_size = 65536;
310+
select * from (
311+
select 'SN0LL' as k from t1
312+
) as a1 left join (select * from t2) as a2 on a1.k = a2.a;
303313

304314
statement ok
305315
DROP TABLE IF EXISTS t1;

0 commit comments

Comments
 (0)