Skip to content

Commit deb7de0

Browse files
authored
Remove unused parameters from vtab functions (#7738)
Signed-off-by: Mikhail Kot <to@myrrc.dev>
1 parent 6d2aee8 commit deb7de0

10 files changed

Lines changed: 108 additions & 261 deletions

File tree

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

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -114,32 +114,24 @@ typedef struct {
114114

115115
duckdb_vx_data (*init_global)(const duckdb_vx_tfunc_init_input *input, duckdb_vx_error *error_out);
116116

117-
duckdb_vx_data (*init_local)(const duckdb_vx_tfunc_init_input *input,
118-
void *init_global_data,
119-
duckdb_vx_error *error_out);
117+
duckdb_vx_data (*init_local)(void *init_global_data);
120118

121-
void (*function)(duckdb_client_context ctx,
122-
const void *bind_data,
123-
void *init_global_data,
119+
void (*function)(void *init_global_data,
124120
void *init_local_data,
125121
duckdb_data_chunk data_chunk_out,
126122
duckdb_vx_error *error_out);
127123

128-
bool (*statistics)(duckdb_client_context context,
129-
const void *bind_data,
130-
size_t column_index,
131-
duckdb_column_statistics *stats_out);
124+
bool (*statistics)(const void *bind_data, size_t column_index, duckdb_column_statistics *stats_out);
132125

133126
void (*cardinality)(void *bind_data, duckdb_vx_node_statistics *node_stats_out);
134127

135128
bool (*pushdown_complex_filter)(void *bind_data, duckdb_vx_expr expr, duckdb_vx_error *error_out);
136129

137130
void (*to_string)(void *bind_data, duckdb_vx_string_map map);
138131

139-
double (*table_scan_progress)(duckdb_client_context ctx, void *bind_data, void *global_state);
132+
double (*table_scan_progress)(void *global_state);
140133

141-
void (*get_partition_data)(const void *bind_data,
142-
void *init_global_data,
134+
void (*get_partition_data)(void *init_global_data,
143135
void *init_local_data,
144136
duckdb_vx_partition_data *partition_data_out);
145137
} duckdb_vx_tfunc_vtab_t;

vortex-duckdb/cpp/table_function.cpp

Lines changed: 15 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,12 @@ struct CTableLocalData final : LocalTableFunctionState {
8282
unique_ptr<CData> ffi_data;
8383
};
8484

85-
double c_table_scan_progress(ClientContext &context,
86-
const FunctionData *bind_data,
87-
const GlobalTableFunctionState *global_state) {
85+
double table_scan_progress(ClientContext &,
86+
const FunctionData *bind_data,
87+
const GlobalTableFunctionState *global_state) {
8888
auto &bind = bind_data->Cast<CTableBindData>();
89-
duckdb_client_context c_ctx = reinterpret_cast<duckdb_client_context>(&context);
90-
void *const c_bind_data = bind.ffi_data->DataPtr();
9189
void *const c_global_state = global_state->Cast<CTableGlobalData>().ffi_data->DataPtr();
92-
return bind.info.vtab.table_scan_progress(c_ctx, c_bind_data, c_global_state);
90+
return bind.info.vtab.table_scan_progress(c_global_state);
9391
}
9492

9593
static Value &UnwrapValue(duckdb_value value) {
@@ -140,18 +138,16 @@ unique_ptr<BaseStatistics> base_stats(duckdb_column_statistics &stats, LogicalTy
140138
return out.ToUnique();
141139
}
142140

143-
unique_ptr<BaseStatistics>
144-
c_statistics(ClientContext &context, const FunctionData *bind_data, column_t column_index) {
141+
unique_ptr<BaseStatistics> statistics(ClientContext &, const FunctionData *bind_data, column_t column_index) {
145142
if (IsVirtualColumn(column_index)) {
146143
return {};
147144
}
148145

149146
const auto &bind = bind_data->Cast<CTableBindData>();
150147
void *const ffi_bind = bind.ffi_data->DataPtr();
151148

152-
duckdb_client_context c_ctx = reinterpret_cast<duckdb_client_context>(&context);
153149
duckdb_column_statistics statistics = {};
154-
if (!bind.info.vtab.statistics(c_ctx, ffi_bind, column_index, &statistics)) {
150+
if (!bind.info.vtab.statistics(ffi_bind, column_index, &statistics)) {
155151
return {};
156152
}
157153

@@ -243,43 +239,25 @@ unique_ptr<GlobalTableFunctionState> c_init_global(ClientContext &context, Table
243239
return make_uniq<CTableGlobalData>(std::move(cdata));
244240
}
245241

246-
unique_ptr<LocalTableFunctionState> c_init_local(ExecutionContext &context,
247-
TableFunctionInitInput &input,
248-
GlobalTableFunctionState *global_state) {
242+
unique_ptr<LocalTableFunctionState>
243+
init_local(ExecutionContext &, TableFunctionInitInput &input, GlobalTableFunctionState *global_state) {
249244
const auto &bind = input.bind_data->Cast<CTableBindData>();
250245
void *const ffi_global = global_state->Cast<CTableGlobalData>().ffi_data->DataPtr();
251246

252-
duckdb_vx_tfunc_init_input ffi_input = {
253-
.bind_data = bind.ffi_data->DataPtr(),
254-
.column_ids = input.column_ids.data(),
255-
.column_ids_count = input.column_ids.size(),
256-
.projection_ids = input.projection_ids.data(),
257-
.projection_ids_count = input.projection_ids.size(),
258-
.filters = reinterpret_cast<duckdb_vx_table_filter_set>(input.filters.get()),
259-
.client_context = reinterpret_cast<duckdb_client_context>(&context),
260-
};
261-
262-
duckdb_vx_error error_out = nullptr;
263-
duckdb_vx_data ffi_local_data = bind.info.vtab.init_local(&ffi_input, ffi_global, &error_out);
264-
if (error_out) {
265-
throw BinderException(IntoErrString(error_out));
266-
}
267-
247+
duckdb_vx_data ffi_local_data = bind.info.vtab.init_local(ffi_global);
268248
auto cdata = unique_ptr<CData>(reinterpret_cast<CData *>(ffi_local_data));
269249
return make_uniq<CTableLocalData>(std::move(cdata));
270250
}
271251

272-
void c_function(ClientContext &context, TableFunctionInput &input, DataChunk &output) {
252+
void function(ClientContext &, TableFunctionInput &input, DataChunk &output) {
273253
const auto &bind = input.bind_data->Cast<CTableBindData>();
274254

275-
duckdb_client_context ffi_ctx = reinterpret_cast<duckdb_client_context>(&context);
276-
void *const ffi_bind = bind.ffi_data->DataPtr();
277255
void *const ffi_global = input.global_state->Cast<CTableGlobalData>().ffi_data->DataPtr();
278256
void *const ffi_local = input.local_state->Cast<CTableLocalData>().ffi_data->DataPtr();
279257

280258
duckdb_data_chunk chunk = reinterpret_cast<duckdb_data_chunk>(&output);
281259
duckdb_vx_error error_out = nullptr;
282-
bind.info.vtab.function(ffi_ctx, ffi_bind, ffi_global, ffi_local, chunk, &error_out);
260+
bind.info.vtab.function(ffi_global, ffi_local, chunk, &error_out);
283261
if (error_out) {
284262
throw InvalidInputException(IntoErrString(error_out));
285263
}
@@ -366,11 +344,10 @@ TablePartitionInfo get_partition_info(ClientContext &, TableFunctionPartitionInp
366344
*/
367345
OperatorPartitionData get_partition_data(ClientContext &, TableFunctionGetPartitionInput &input) {
368346
auto &bind = input.bind_data->Cast<CTableBindData>();
369-
void *const ffi_bind = bind.ffi_data->DataPtr();
370347
void *const ffi_global = input.global_state->Cast<CTableGlobalData>().ffi_data->DataPtr();
371348
void *const ffi_local = input.local_state->Cast<CTableLocalData>().ffi_data->DataPtr();
372349
duckdb_vx_partition_data partition_data;
373-
bind.info.vtab.get_partition_data(ffi_bind, ffi_global, ffi_local, &partition_data);
350+
bind.info.vtab.get_partition_data(ffi_global, ffi_local, &partition_data);
374351

375352
OperatorPartitionData out(partition_data.partition_index);
376353

@@ -410,7 +387,7 @@ extern "C" duckdb_state duckdb_vx_tfunc_register(duckdb_database ffi_db, const d
410387

411388
const DatabaseWrapper &wrapper = *reinterpret_cast<DatabaseWrapper *>(ffi_db);
412389
DatabaseInstance &db = *wrapper.database->instance;
413-
TableFunction tf(vtab->name, {}, c_function, c_bind, c_init_global, c_init_local);
390+
TableFunction tf(vtab->name, {}, function, c_bind, c_init_global, init_local);
414391

415392
tf.projection_pushdown = true;
416393
tf.filter_pushdown = true;
@@ -422,8 +399,8 @@ extern "C" duckdb_state duckdb_vx_tfunc_register(duckdb_database ffi_db, const d
422399
tf.get_partition_info = get_partition_info;
423400
tf.get_partition_data = get_partition_data;
424401
tf.to_string = c_to_string;
425-
tf.table_scan_progress = c_table_scan_progress;
426-
tf.statistics = c_statistics;
402+
tf.table_scan_progress = table_scan_progress;
403+
tf.statistics = statistics;
427404

428405
tf.late_materialization = true;
429406
// Columns that uniquely identify a row for deferred re-fetch in a multi

vortex-duckdb/src/datasource.rs

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -254,10 +254,6 @@ impl ColumnStatisticsAggregate {
254254
}
255255
}
256256

257-
// ---------------------------------------------------------------------------
258-
// Blanket TableFunction implementation for any DataSourceTableFunction
259-
// ---------------------------------------------------------------------------
260-
261257
impl<T: DataSourceTableFunction> TableFunction for T {
262258
type BindData = DataSourceBindData;
263259
type GlobalState = DataSourceGlobal;
@@ -398,10 +394,7 @@ impl<T: DataSourceTableFunction> TableFunction for T {
398394
})
399395
}
400396

401-
fn init_local(
402-
_init: &TableInitInput<Self>,
403-
global: &Self::GlobalState,
404-
) -> VortexResult<Self::LocalState> {
397+
fn init_local(global: &Self::GlobalState) -> Self::LocalState {
405398
unsafe {
406399
use custom_labels::sys;
407400

@@ -417,17 +410,15 @@ impl<T: DataSourceTableFunction> TableFunction for T {
417410
CURRENT_LABELSET.set(key, value);
418411
}
419412

420-
Ok(DataSourceLocal {
413+
DataSourceLocal {
421414
iterator: global.iterator.clone(),
422415
exporter: None,
423416
partition_index: 0,
424417
file_index: 0,
425-
})
418+
}
426419
}
427420

428421
fn scan(
429-
_client_context: &ClientContextRef,
430-
_bind_data: &Self::BindData,
431422
local_state: &mut Self::LocalState,
432423
global_state: &Self::GlobalState,
433424
chunk: &mut DataChunkRef,
@@ -501,11 +492,7 @@ impl<T: DataSourceTableFunction> TableFunction for T {
501492
Ok(())
502493
}
503494

504-
fn table_scan_progress(
505-
_client_context: &ClientContextRef,
506-
_bind_data: &Self::BindData,
507-
global_state: &Self::GlobalState,
508-
) -> f64 {
495+
fn table_scan_progress(global_state: &Self::GlobalState) -> f64 {
509496
progress(&global_state.bytes_read, &global_state.bytes_total)
510497
}
511498

@@ -532,11 +519,7 @@ impl<T: DataSourceTableFunction> TableFunction for T {
532519

533520
/// Get column-wise statistics. Available only if we're reading a single
534521
/// file.
535-
fn statistics(
536-
_client_context: &ClientContextRef,
537-
bind_data: &Self::BindData,
538-
column_index: usize,
539-
) -> Option<ColumnStatistics> {
522+
fn statistics(bind_data: &Self::BindData, column_index: usize) -> Option<ColumnStatistics> {
540523
let children = bind_data.data_source.children();
541524
// Otherwise we'd have to open all files eagerly which is a performance
542525
// regression. Duckdb's Parquet reader only gets metadata for multiple
@@ -566,7 +549,6 @@ impl<T: DataSourceTableFunction> TableFunction for T {
566549
}
567550

568551
fn partition_data(
569-
_bind_data: &Self::BindData,
570552
global_init_data: &Self::GlobalState,
571553
local_init_data: &mut Self::LocalState,
572554
) -> PartitionData {
@@ -586,10 +568,6 @@ impl<T: DataSourceTableFunction> TableFunction for T {
586568
}
587569
}
588570

589-
// ---------------------------------------------------------------------------
590-
// Helper functions
591-
// ---------------------------------------------------------------------------
592-
593571
/// Extracts DuckDB column names and logical types from a Vortex struct DType.
594572
fn extract_schema_from_dtype(dtype: &DType) -> VortexResult<Vec<DuckdbField>> {
595573
let struct_dtype = dtype

vortex-duckdb/src/duckdb/table_function/init.rs

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,26 +40,13 @@ pub(crate) unsafe extern "C-unwind" fn init_global_callback<T: TableFunction>(
4040

4141
/// Native callback for the local initialization of a table function.
4242
pub(crate) unsafe extern "C-unwind" fn init_local_callback<T: TableFunction>(
43-
init_input: *const cpp::duckdb_vx_tfunc_init_input,
4443
global_init_data: *mut c_void,
45-
error_out: *mut cpp::duckdb_vx_error,
4644
) -> cpp::duckdb_vx_data {
47-
let init_input = TableInitInput::new(
48-
unsafe { init_input.as_ref() }.vortex_expect("init_input null pointer"),
49-
);
50-
5145
let global_init_data = unsafe { global_init_data.cast::<T::GlobalState>().as_ref() }
5246
.vortex_expect("global_init_data null pointer");
5347

54-
match T::init_local(&init_input, global_init_data) {
55-
Ok(init_data) => Data::from(Box::new(init_data)).as_ptr(),
56-
Err(e) => {
57-
// Set the error in the error output.
58-
let msg = e.to_string();
59-
unsafe { error_out.write(cpp::duckdb_vx_error_create(msg.as_ptr().cast(), msg.len())) };
60-
ptr::null_mut::<cpp::duckdb_vx_data_>().cast()
61-
}
62-
}
48+
let init_data = T::init_local(global_init_data);
49+
Data::from(Box::new(init_data)).as_ptr()
6350
}
6451

6552
/// A typed wrapper for the input to a table function's initialization.

0 commit comments

Comments
 (0)