Skip to content

Commit ee01b02

Browse files
committed
Updated duckdb client to read the nested stats.
1 parent 70fffc1 commit ee01b02

6 files changed

Lines changed: 132 additions & 73 deletions

File tree

‎vortex-duckdb/cpp/copy_function.cpp‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include "duckdb/main/capi/capi_internal.hpp"
1212
#include "duckdb/main/client_context.hpp"
1313
#include "duckdb/main/connection.hpp"
14-
#include "duckdb/parser/keyword_helper.hpp"
1514
#include "duckdb/parser/parsed_data/create_copy_function_info.hpp"
1615

1716
unique_ptr<FunctionData> copy_to_bind(ClientContext &,
@@ -40,7 +39,7 @@ unique_ptr<FunctionData> copy_to_bind(ClientContext &,
4039
throw BinderException(IntoErrString(error_out));
4140
}
4241
auto cdata = unique_ptr<CData>(reinterpret_cast<CData *>(ffi_bind_data));
43-
return make_uniq<VortexCopyBindData>(std::move(cdata), names);
42+
return make_uniq<VortexCopyBindData>(std::move(cdata));
4443
}
4544

4645
unique_ptr<GlobalFunctionData>
@@ -86,7 +85,7 @@ void copy_to_get_written_statistics(ClientContext &,
8685
gstate.Cast<VortexCopyGlobalState>().written_stats = &statistics;
8786
}
8887

89-
void copy_to_finalize(ClientContext &, FunctionData &bind_data, GlobalFunctionData &gstate) {
88+
void copy_to_finalize(ClientContext &, FunctionData &, GlobalFunctionData &gstate) {
9089
auto &global = gstate.Cast<VortexCopyGlobalState>();
9190
void *const ffi_global = global.ffi_global->DataPtr();
9291
duckdb_vx_error error_out = nullptr;
@@ -98,25 +97,18 @@ void copy_to_finalize(ClientContext &, FunctionData &bind_data, GlobalFunctionDa
9897
if (!global.written_stats) {
9998
return;
10099
}
101-
auto &names = bind_data.Cast<VortexCopyBindData>().column_names;
102100
duckdb_vx_written_file_statistics file_stats;
103101
if (!duckdb_copy_function_get_written_file_statistics(ffi_global, &file_stats)) {
104102
// Statistics were requested (written_stats is set) but the finished write produced none;
105103
// that is an internal inconsistency, not a silently empty result.
106104
throw InternalException("vortex COPY: written statistics were requested but not produced");
107105
}
108-
if (file_stats.num_columns != names.size()) {
109-
throw InternalException("vortex COPY: %llu statistics columns for %llu written columns",
110-
file_stats.num_columns,
111-
names.size());
112-
}
113106
D_ASSERT(global.written_stats != nullptr);
114107
global.written_stats->row_count = file_stats.row_count;
115108
global.written_stats->file_size_bytes = file_stats.file_size_bytes;
116109
global.written_stats->footer_size_bytes = Value::UBIGINT(file_stats.footer_size_bytes);
117-
// Keyed by top-level column name only. The vortex footer reports one statistics set per
118-
// top-level field, so nested struct/list leaf columns get no statistics here (unlike parquet,
119-
// which recurses to leaf paths). Flat tables are fully covered.
110+
// One entry per leaf column: nested struct fields are reported at their full path and structs
111+
// get no entry of their own, as in parquet.
120112
for (idx_t i = 0; i < file_stats.num_columns; i++) {
121113
duckdb_vx_written_column_statistics col_stats {};
122114
duckdb_vx_error col_error = nullptr;
@@ -145,10 +137,9 @@ void copy_to_finalize(ClientContext &, FunctionData &bind_data, GlobalFunctionDa
145137
if (col_stats.has_nan_stat) {
146138
column["has_nan"] = Value::BOOLEAN(col_stats.contains_nan);
147139
}
148-
// DuckLake keys column statistics by a quoted, dot-separated path (see
149-
// DuckLakeUtil::ParseQuotedList); match the parquet writer, which quotes each name.
150-
global.written_stats->column_statistics.emplace(KeywordHelper::WriteQuoted(names[i], '"'),
151-
std::move(column));
140+
auto column_key = reinterpret_cast<Value *>(col_stats.column_key)->ToString();
141+
duckdb_destroy_value(&col_stats.column_key);
142+
global.written_stats->column_statistics.emplace(std::move(column_key), std::move(column));
152143
}
153144
}
154145

‎vortex-duckdb/cpp/include/copy_function.hpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,9 @@
88
using namespace duckdb;
99

1010
struct VortexCopyBindData final : TableFunctionData {
11-
VortexCopyBindData(unique_ptr<CData> ffi_bind, vector<string> column_names)
12-
: ffi_bind(std::move(ffi_bind)), column_names(std::move(column_names)) {
11+
VortexCopyBindData(unique_ptr<CData> ffi_bind) : ffi_bind(std::move(ffi_bind)) {
1312
}
1413
unique_ptr<CData> ffi_bind;
15-
// Column names in write order, used to key WRITTEN_FILE_STATISTICS.
16-
vector<string> column_names;
1714
};
1815

1916
struct VortexCopyGlobalState final : GlobalFunctionData {

‎vortex-duckdb/cpp/include/table_function.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ typedef struct {
8383

8484
// Per-column statistics of a written Vortex file.
8585
typedef struct {
86+
// Owned VARCHAR value holding the column's quoted, dot-separated path (e.g. `"s"."b"`), the key
87+
// format DuckLake expects; the caller must destroy it.
88+
duckdb_value column_key;
8689
// Owned values, null if absent; the caller must destroy them.
8790
duckdb_value min;
8891
duckdb_value max;

‎vortex-duckdb/src/copy.rs‎

Lines changed: 116 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ use vortex::error::vortex_bail;
2626
use vortex::error::vortex_err;
2727
use vortex::expr::stats::Precision;
2828
use vortex::expr::stats::Stat;
29+
use vortex::file::CompressedFieldSizes;
30+
use vortex::file::FileStatistics;
2931
use vortex::file::WriteOptionsSessionExt;
3032
use vortex::file::WriteSummary;
3133
use vortex::file::multi::parse_uri_or_path;
@@ -55,11 +57,13 @@ pub struct CopyFunctionBind {
5557
}
5658
assert_impl_all!(CopyFunctionBind: Send, Clone);
5759

58-
/// The per-column compressed sizes are computed once here rather than per column, since DuckDB
59-
/// queries statistics one column at a time.
60+
/// The compressed field sizes and the statistics columns are computed once here rather than per
61+
/// column, since DuckDB queries statistics one column at a time.
6062
struct FinishedWrite {
6163
summary: WriteSummary,
62-
column_sizes: Vec<u64>,
64+
column_sizes: Option<CompressedFieldSizes>,
65+
/// Indices into the footer's [`FileStatistics`] of the entries reported as statistics columns.
66+
stats_columns: Vec<usize>,
6367
}
6468

6569
/// Write to a file has two phases, writing data chunks and then closing the file.
@@ -160,12 +164,18 @@ pub fn copy_to_finalize(init_global: &mut CopyFunctionGlobal) -> VortexResult<()
160164
.take()
161165
.vortex_expect("no file to close");
162166
// Keep the write summary (footer + size) so DuckLake can read per-file statistics back
163-
// without re-opening the file. Compute the per-column compressed sizes once, up front.
167+
// without re-opening the file. Compute the compressed field sizes once, up front.
164168
let summary = task.await?;
165-
let column_sizes = summary.compressed_column_sizes().unwrap_or_default();
169+
let column_sizes = summary.footer().compressed_field_sizes().ok();
170+
let stats_columns = summary
171+
.footer()
172+
.statistics()
173+
.map(leaf_stats_columns)
174+
.unwrap_or_default();
166175
*init_global.finished.lock() = Some(FinishedWrite {
167176
summary,
168177
column_sizes,
178+
stats_columns,
169179
});
170180
Ok(())
171181
})
@@ -182,6 +192,8 @@ pub(crate) struct WrittenFileStats {
182192
/// Per-column statistics of the written Vortex file. `min`/`max` are DuckDB values converted from
183193
/// the Vortex scalar; every field is optional and omitted when the statistic is not available.
184194
pub(crate) struct WrittenColumnStats {
195+
/// The column's path in DuckLake's key format (see [`ducklake_column_key`]).
196+
pub column_key: String,
185197
pub min: Option<Value>,
186198
pub max: Option<Value>,
187199
pub null_count: Option<u64>,
@@ -193,7 +205,11 @@ pub(crate) struct WrittenColumnStats {
193205
/// Read file-level statistics back from the finished write. `None` before finalize.
194206
pub(crate) fn written_file_stats(global: &CopyFunctionGlobal) -> Option<WrittenFileStats> {
195207
let guard = global.finished.lock();
196-
Some(file_stats_from_summary(&guard.as_ref()?.summary))
208+
let finished = guard.as_ref()?;
209+
Some(file_stats_from_summary(
210+
&finished.summary,
211+
finished.stats_columns.len(),
212+
))
197213
}
198214

199215
/// Read per-column statistics for `column_index` from the finished write. `Ok(None)` if the file is
@@ -206,15 +222,45 @@ pub(crate) fn written_column_stats(
206222
let Some(finished) = guard.as_ref() else {
207223
return Ok(None);
208224
};
209-
column_stats_from_summary(&finished.summary, column_index, &finished.column_sizes).map(Some)
225+
column_stats_from_summary(
226+
&finished.summary,
227+
&finished.stats_columns,
228+
column_index,
229+
finished.column_sizes.as_ref(),
230+
)
231+
.map(Some)
210232
}
211233

212-
fn file_stats_from_summary(summary: &WriteSummary) -> WrittenFileStats {
213-
let num_columns = summary
214-
.footer()
215-
.dtype()
216-
.as_struct_fields_opt()
217-
.map_or(1, StructFields::nfields);
234+
/// The indices of the leaf entries of `file_stats`, which are the columns statistics are reported
235+
/// for. Like parquet, which reports one entry per leaf column writer, struct columns get no entry of
236+
/// their own; nested fields are reported at their full path. Lists and maps are opaque leaves of the
237+
/// statistics layout, so they are reported at their own path rather than per element.
238+
fn leaf_stats_columns(file_stats: &FileStatistics) -> Vec<usize> {
239+
file_stats
240+
.dtypes()
241+
.iter()
242+
.enumerate()
243+
.filter(|(_, dtype)| !dtype.is_struct())
244+
.map(|(index, _)| index)
245+
.collect()
246+
}
247+
248+
/// DuckLake keys column statistics by a quoted, dot-separated path (see
249+
/// `DuckLakeUtil::ParseQuotedList`). This matches the parquet writer, which quotes each name with
250+
/// `KeywordHelper::WriteQuoted(name, '"')`, escaping embedded quotes by doubling them. A list
251+
/// element is named `element`, as in parquet's list encoding.
252+
fn ducklake_column_key(path: &FieldPath) -> String {
253+
path.parts()
254+
.iter()
255+
.map(|part| {
256+
let name = part.as_name().unwrap_or("element");
257+
format!("\"{}\"", name.replace('"', "\"\""))
258+
})
259+
.collect::<Vec<_>>()
260+
.join(".")
261+
}
262+
263+
fn file_stats_from_summary(summary: &WriteSummary, num_columns: usize) -> WrittenFileStats {
218264
WrittenFileStats {
219265
row_count: summary.row_count(),
220266
file_size_bytes: summary.size(),
@@ -224,45 +270,38 @@ fn file_stats_from_summary(summary: &WriteSummary) -> WrittenFileStats {
224270
}
225271
}
226272

227-
/// Per-column statistics from a finished write's summary and its precomputed compressed sizes
228-
/// (`column_sizes`, indexed by top-level field position).
229-
///
230-
/// Only top-level columns are covered: nested struct/list leaf columns are not reported (parquet,
231-
/// by contrast, recurses to leaf paths). Flat tables - the common DuckLake case - are fully
232-
/// covered.
273+
/// Statistics for the `column_index`-th statistics column of a finished write, where
274+
/// `stats_columns` are the [leaf entries][leaf_stats_columns] of its footer statistics and
275+
/// `column_sizes` its compressed field sizes.
233276
fn column_stats_from_summary(
234277
summary: &WriteSummary,
278+
stats_columns: &[usize],
235279
column_index: usize,
236-
column_sizes: &[u64],
280+
column_sizes: Option<&CompressedFieldSizes>,
237281
) -> VortexResult<WrittenColumnStats> {
238282
let file_stats = summary
239283
.footer()
240284
.statistics()
241285
.ok_or_else(|| vortex_err!("written file has no statistics"))?;
242-
let struct_fields = summary
243-
.footer()
244-
.dtype()
245-
.as_struct_fields_opt()
246-
.ok_or_else(|| vortex_err!("written file dtype is not a struct"))?;
247-
let name = struct_fields.names().get(column_index).ok_or_else(|| {
286+
let stats_index = *stats_columns.get(column_index).ok_or_else(|| {
248287
vortex_err!(
249-
"column index {column_index} out of range for {} top-level fields",
250-
struct_fields.nfields()
288+
"column index {column_index} out of range for {} statistics columns",
289+
stats_columns.len()
251290
)
252291
})?;
253-
let (stats, dtype) = file_stats
254-
.get_by_path(&FieldPath::from_name(name.clone()))
255-
.ok_or_else(|| vortex_err!("no statistics for column {name}"))?;
292+
let (stats, dtype) = file_stats.get(stats_index);
293+
let path = &file_stats.paths()[stats_index];
256294

257295
Ok(WrittenColumnStats {
296+
column_key: ducklake_column_key(path),
258297
min: exact_scalar_to_duckdb(stats.get(Stat::Min), dtype)?,
259298
max: exact_scalar_to_duckdb(stats.get(Stat::Max), dtype)?,
260299
null_count: exact_u64(stats.get(Stat::NullCount)),
261300
// NaNCount is exact only for float columns, so this is emitted just for them (as in parquet).
262301
has_nan: exact_u64(stats.get(Stat::NaNCount)).map(|count| count > 0),
263302
num_values: summary.row_count(),
264303
// On-disk compressed size; excludes bytes not attributable to a column (e.g. struct validity).
265-
column_size_bytes: column_sizes.get(column_index).copied(),
304+
column_size_bytes: column_sizes.and_then(|sizes| sizes.get(path)),
266305
})
267306
}
268307

@@ -376,44 +415,71 @@ mod tests {
376415
})
377416
}
378417

418+
/// Statistics for the `column_index`-th statistics column of `summary`, resolved the same way
419+
/// `copy_to_finalize` and [`written_column_stats`] do.
420+
fn column_stats(
421+
summary: &WriteSummary,
422+
column_index: usize,
423+
) -> VortexResult<WrittenColumnStats> {
424+
let stats_columns = summary
425+
.footer()
426+
.statistics()
427+
.map(leaf_stats_columns)
428+
.unwrap_or_default();
429+
let column_sizes = summary.footer().compressed_field_sizes()?;
430+
column_stats_from_summary(summary, &stats_columns, column_index, Some(&column_sizes))
431+
}
432+
379433
#[test]
380434
fn column_stats_out_of_range_is_an_error() {
381435
let summary = write_summary(pruning_aggregate_fns());
382-
assert!(column_stats_from_summary(&summary, 0, &[]).is_ok());
383-
assert!(column_stats_from_summary(&summary, 1, &[]).is_err());
436+
assert!(column_stats(&summary, 0).is_ok());
437+
assert!(column_stats(&summary, 1).is_err());
384438
}
385439

386440
#[test]
387441
fn column_stats_without_file_statistics_is_an_error() {
388442
let summary = write_summary(vec![]);
389-
assert!(column_stats_from_summary(&summary, 0, &[]).is_err());
443+
assert!(column_stats(&summary, 0).is_err());
390444
}
391445

392446
#[test]
393-
fn column_stats_resolves_by_name_past_a_nullable_struct_column() {
394-
// Regression test: a nullable top-level struct field ("s") contributes both a `s.b` entry
395-
// and a trailing entry for its own null count to the nested (post-order) stats layout, so
396-
// that layout has one more entry than there are top-level fields. Resolving a later
397-
// top-level column ("c", index 1) by flat position into that layout would silently return
398-
// `s`'s own null-count-only entry (which has no Min) instead of `c`'s stats.
447+
fn column_stats_reports_nested_leaves_by_quoted_path() -> VortexResult<()> {
448+
// A nullable struct `s` contributes `s.b` and a trailing own entry to the post-order stats
449+
// layout; only the leaves are statistics columns, keyed by their full quoted path.
399450
let summary = RUNTIME.block_on(async {
400451
let b = buffer![10i32, 20, 30].into_array();
401452
let inner = StructArray::new(["b"].into(), [b], 3, Validity::AllValid).into_array();
402453
let c = buffer![7u32, 8, 9].into_array();
403-
let outer = StructArray::new(["s", "c"].into(), [inner, c], 3, Validity::NonNullable)
404-
.into_array();
454+
let outer =
455+
StructArray::new(["s", "c\"q"].into(), [inner, c], 3, Validity::NonNullable)
456+
.into_array();
405457

406458
let mut buf = ByteBufferMut::empty();
407459
let mut writer = SESSION
408460
.write_options()
409461
.with_file_statistics(pruning_aggregate_fns())
410462
.writer(&mut buf, outer.dtype().clone());
411-
writer.push(outer).await.unwrap();
412-
writer.finish().await.unwrap()
413-
});
414-
415-
let stats = column_stats_from_summary(&summary, 1, &[]).unwrap();
416-
assert!(stats.min.is_some());
417-
assert!(stats.max.is_some());
463+
writer.push(outer).await?;
464+
writer.finish().await
465+
})?;
466+
467+
let file_stats = summary
468+
.footer()
469+
.statistics()
470+
.ok_or_else(|| vortex_err!("no statistics"))?;
471+
assert_eq!(leaf_stats_columns(file_stats).len(), 2);
472+
let nested = column_stats(&summary, 0)?;
473+
assert_eq!(nested.column_key, "\"s\".\"b\"");
474+
assert!(nested.min.is_some());
475+
assert!(nested.column_size_bytes.is_some_and(|size| size > 0));
476+
477+
let top_level = column_stats(&summary, 1)?;
478+
assert_eq!(top_level.column_key, "\"c\"\"q\"");
479+
assert!(top_level.min.is_some());
480+
assert!(top_level.max.is_some());
481+
482+
assert!(column_stats(&summary, 2).is_err());
483+
Ok(())
418484
}
419485
}

‎vortex-duckdb/src/ffi.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use crate::duckdb::Expression;
3333
use crate::duckdb::LogicalType;
3434
use crate::duckdb::LogicalTypeRef;
3535
use crate::duckdb::TableInitInput;
36+
use crate::duckdb::Value;
3637
use crate::duckdb::try_or;
3738
use crate::duckdb::try_or_null;
3839
use crate::file_reader::OpenFileReader;
@@ -495,6 +496,7 @@ pub unsafe extern "C-unwind" fn duckdb_copy_function_get_written_column_statisti
495496
return Ok(false);
496497
};
497498
let out = unsafe { &mut *out };
499+
out.column_key = Value::from(stats.column_key.as_str()).into_ptr();
498500
out.min = stats.min.map_or(ptr::null_mut(), |v| v.into_ptr());
499501
out.max = stats.max.map_or(ptr::null_mut(), |v| v.into_ptr());
500502
out.has_null_count = stats.null_count.is_some();

‎vortex-sqllogictest/slt/duckdb/copy_return_stats.slt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ TO '${WORK_DIR}/copy_return_stats.vortex' (FORMAT vortex, RETURN_STATS);
1111
----
1212
<REGEX>:\$\{WORK_DIR\}/copy_return_stats\.vortex 2 \d+ 0 \{'"d"'=\{column_size_bytes=\d+, max=1\.5, min=1\.5, null_count=1, num_values=2\}, '"i"'=\{column_size_bytes=\d+, max=2, min=1, null_count=0, num_values=2\}, '"s"'=\{column_size_bytes=\d+, max=b, min=a, null_count=0, num_values=2\}\} NULL
1313

14-
# Vortex reports statistics per top-level field, so nested columns are covered by the file-level
15-
# counts only; the COPY must still succeed.
14+
# As in parquet, statistics are reported per leaf column: struct fields are keyed by their full
15+
# quoted path and the struct itself gets no entry. Lists are reported as a single column.
1616
query TIII??
1717
COPY (SELECT {'a': 1, 'b': 2} AS st, [1, 2, 3] AS lst, 7 AS i)
1818
TO '${WORK_DIR}/copy_return_stats_nested.vortex' (FORMAT vortex, RETURN_STATS);
1919
----
20-
<REGEX>:\$\{WORK_DIR\}/copy_return_stats_nested\.vortex 1 \d+ 0 \{'"i"'=\{column_size_bytes=\d+, max=7, min=7, null_count=0, num_values=1\}, '"lst"'=\{column_size_bytes=\d+, null_count=0, num_values=1\}, '"st"'=\{column_size_bytes=\d+, null_count=0, num_values=1\}\} NULL
20+
<REGEX>:\$\{WORK_DIR\}/copy_return_stats_nested\.vortex 1 \d+ 0 \{'"i"'=\{column_size_bytes=\d+, max=7, min=7, null_count=0, num_values=1\}, '"lst"'=\{column_size_bytes=\d+, null_count=0, num_values=1\}, '"st"\."a"'=\{column_size_bytes=\d+, max=1, min=1, null_count=0, num_values=1\}, '"st"\."b"'=\{column_size_bytes=\d+, max=2, min=2, null_count=0, num_values=1\}\} NULL

0 commit comments

Comments
 (0)