Skip to content

fix: Don't nullify release callback for schemas #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
peasee merged 6 commits into from
Oct 21, 2025
Merged

fix: Don't nullify release callback for schemas #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2c587c5
fix: Don't nullify release callback for schemas
peasee Oct 21, 2025
c6c8e1c
chore: Format
peasee Oct 21, 2025
3d9720d
wip
peasee Oct 21, 2025
03db40f
wip
peasee Oct 21, 2025
0ee0f93
chore: format
peasee Oct 21, 2025
14fa148
wip
peasee Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/function/table/arrow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ void ArrowTableFunction::PopulateArrowTableType(DBConfig &config, ArrowTableType
const ArrowSchemaWrapper &schema_p, vector<string> &names,
vector<LogicalType> &return_types) {
for (idx_t col_idx = 0; col_idx < static_cast<idx_t>(schema_p.arrow_schema.n_children); col_idx++) {
if (schema_p.arrow_schema.children[col_idx] == nullptr) {
throw InvalidInputException("arrow_scan: null schema pointer for column %d", col_idx);
}

auto &schema = *schema_p.arrow_schema.children[col_idx];
if (!schema.release) {
throw InvalidInputException("arrow_scan: released schema passed");
Expand Down Expand Up @@ -74,6 +78,7 @@ unique_ptr<FunctionData> ArrowTableFunction::ArrowScanBind(ClientContext &contex
if (return_types.empty()) {
throw InvalidInputException("Provided table/dataframe must have at least one column");
}

return std::move(res);
}

Expand Down
52 changes: 27 additions & 25 deletions src/function/table/table_scan.cpp
View file
Open in desktop
Copy link
Copy Markdown
Author

@peasee peasee Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a formatting change

Original file line number Diff line number Diff line change
Expand Up @@ -490,36 +490,38 @@ vector<unique_ptr<Expression>> ExtractFilterExpressions(const ColumnDefinition &
// Recursively updates column bindings in an index expression to match the current input projection,
// even if they are the child of a function or cast
void UpdateIndexExprColumnBindings(unique_ptr<Expression> &expr, const vector<column_t> &input_column_ids,
const vector<column_t> &indexed_columns) {
if (!expr) { return; }
const vector<column_t> &indexed_columns) {
if (!expr) {
return;
}

switch (expr->GetExpressionClass()) {
case ExpressionClass::BOUND_COLUMN_REF: {
auto &bound_column_ref_expr = expr->Cast<BoundColumnRefExpression>();

for (idx_t i=0; i < input_column_ids.size(); ++i) {
if (input_column_ids[i] == indexed_columns[0]) {
bound_column_ref_expr.binding.column_index = i;
break;
}
case ExpressionClass::BOUND_COLUMN_REF: {
auto &bound_column_ref_expr = expr->Cast<BoundColumnRefExpression>();

for (idx_t i = 0; i < input_column_ids.size(); ++i) {
if (input_column_ids[i] == indexed_columns[0]) {
bound_column_ref_expr.binding.column_index = i;
break;
}
break;
}
case ExpressionClass::BOUND_FUNCTION: {
auto &func_expr = expr->Cast<BoundFunctionExpression>();
for (auto &child : func_expr.children) {
UpdateIndexExprColumnBindings(child, input_column_ids, indexed_columns);
}
break;
break;
}
case ExpressionClass::BOUND_FUNCTION: {
auto &func_expr = expr->Cast<BoundFunctionExpression>();
for (auto &child : func_expr.children) {
UpdateIndexExprColumnBindings(child, input_column_ids, indexed_columns);
}
// TODO: never end up here, TableScanInitGlobal called without filters in input
// case ExpressionClass::BOUND_CAST: {
// auto &cast_expr = expr->Cast<BoundCastExpression>();
// UpdateIndexExprColumnBindings(cast_expr.child, input_column_ids, indexed_columns);
// break;
// }
default:
break;
break;
}
// TODO: never end up here, TableScanInitGlobal called without filters in input
// case ExpressionClass::BOUND_CAST: {
// auto &cast_expr = expr->Cast<BoundCastExpression>();
// UpdateIndexExprColumnBindings(cast_expr.child, input_column_ids, indexed_columns);
// break;
// }
default:
break;
}
}

Expand Down
15 changes: 7 additions & 8 deletions src/main/capi/arrow-c.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,6 @@ void EmptyStreamRelease(ArrowArrayStream *stream) {

void FactoryGetSchema(ArrowArrayStream *stream, ArrowSchema &schema) {
stream->get_schema(stream, &schema);

// Need to nullify the root schema's release function here, because streams don't allow us to set the release
// function. For the schema's children, we nullify the release functions in `duckdb_arrow_scan`, so we don't need to
// handle them again here. We set this to nullptr and not EmptySchemaRelease to prevent ArrowSchemaWrapper's
// destructor from destroying the schema (it's the caller's responsibility).
schema.release = nullptr;
}

int GetSchema(struct ArrowArrayStream *stream, struct ArrowSchema *out) {
Expand All @@ -218,7 +212,6 @@ int GetSchema(struct ArrowArrayStream *stream, struct ArrowSchema *out) {
}

*out = *private_data->schema;
out->release = EmptySchemaRelease;
return DuckDBSuccess;
}

Expand Down Expand Up @@ -254,7 +247,13 @@ const char *GetLastError(struct ArrowArrayStream *stream) {

void Release(struct ArrowArrayStream *stream) {
if (stream->private_data != nullptr) {
delete reinterpret_cast<PrivateData *>(stream->private_data);
auto private_data = reinterpret_cast<PrivateData *>(stream->private_data);
if (private_data->schema && private_data->schema->release) {
private_data->schema->release(private_data->schema);
private_data->schema = nullptr;
}

delete private_data;
}

stream->private_data = nullptr;
Expand Down
Loading