Skip to content

Commit 94af11b

Browse files
committed
Revert "Run ArrowConverter::ToArrowSchema in a transaction"
This reverts commit 7e8dbc6.
1 parent 881ab6d commit 94af11b

3 files changed

Lines changed: 27 additions & 80 deletions

File tree

src/common/arrow/arrow_converter.cpp

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -417,42 +417,37 @@ void ArrowConverter::ToArrowSchema(ArrowSchema *out_schema, const vector<Logical
417417
const vector<string> &names, ClientProperties &options) {
418418
D_ASSERT(out_schema);
419419
D_ASSERT(types.size() == names.size());
420-
D_ASSERT(options.client_context);
421-
// We need to run this in a transaction. The arrow schema callback might use the catalog to do lookups.
422-
options.client_context->RunFunctionInTransaction([&types, &out_schema, &names, &options]() {
423-
const idx_t column_count = types.size();
424-
// Allocate as unique_ptr first to clean-up properly on error
425-
auto root_holder = make_uniq<DuckDBArrowSchemaHolder>();
420+
const idx_t column_count = types.size();
421+
// Allocate as unique_ptr first to clean-up properly on error
422+
auto root_holder = make_uniq<DuckDBArrowSchemaHolder>();
426423

427-
// Allocate the children
428-
root_holder->children.resize(column_count);
429-
root_holder->children_ptrs.resize(column_count, nullptr);
430-
for (size_t i = 0; i < column_count; ++i) {
431-
root_holder->children_ptrs[i] = &root_holder->children[i];
432-
}
433-
out_schema->children = root_holder->children_ptrs.data();
434-
out_schema->n_children = NumericCast<int64_t>(column_count);
424+
// Allocate the children
425+
root_holder->children.resize(column_count);
426+
root_holder->children_ptrs.resize(column_count, nullptr);
427+
for (size_t i = 0; i < column_count; ++i) {
428+
root_holder->children_ptrs[i] = &root_holder->children[i];
429+
}
430+
out_schema->children = root_holder->children_ptrs.data();
431+
out_schema->n_children = NumericCast<int64_t>(column_count);
435432

436-
// Store the schema
437-
out_schema->format = "+s"; // struct apparently
438-
out_schema->flags = 0;
439-
out_schema->metadata = nullptr;
440-
out_schema->name = "duckdb_query_result";
441-
out_schema->dictionary = nullptr;
433+
// Store the schema
434+
out_schema->format = "+s"; // struct apparently
435+
out_schema->flags = 0;
436+
out_schema->metadata = nullptr;
437+
out_schema->name = "duckdb_query_result";
438+
out_schema->dictionary = nullptr;
442439

443-
// Configure all child schemas
444-
for (idx_t col_idx = 0; col_idx < column_count; col_idx++) {
445-
root_holder->owned_column_names.push_back(AddName(names[col_idx]));
446-
auto &child = root_holder->children[col_idx];
447-
InitializeChild(child, *root_holder, names[col_idx]);
448-
SetArrowFormat(*root_holder, child, types[col_idx], options, *options.client_context);
449-
}
450-
451-
// Release ownership to caller
452-
out_schema->private_data = root_holder.release();
453-
out_schema->release = ReleaseDuckDBArrowSchema;
454-
});
440+
// Configure all child schemas
441+
for (idx_t col_idx = 0; col_idx < column_count; col_idx++) {
442+
root_holder->owned_column_names.push_back(AddName(names[col_idx]));
443+
auto &child = root_holder->children[col_idx];
444+
InitializeChild(child, *root_holder, names[col_idx]);
445+
SetArrowFormat(*root_holder, child, types[col_idx], options, *options.client_context);
446+
}
455447

448+
// Release ownership to caller
449+
out_schema->private_data = root_holder.release();
450+
out_schema->release = ReleaseDuckDBArrowSchema;
456451
}
457452

458453
} // namespace duckdb

src/main/capi/arrow-c.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ using duckdb::ArrowResultWrapper;
1010
using duckdb::CClientArrowOptionsWrapper;
1111
using duckdb::Connection;
1212
using duckdb::DataChunk;
13-
using duckdb::ErrorData;
1413
using duckdb::LogicalType;
1514
using duckdb::MaterializedQueryResult;
1615
using duckdb::PreparedStatementWrapper;
@@ -181,11 +180,7 @@ duckdb_state duckdb_query_arrow_schema(duckdb_arrow result, duckdb_arrow_schema
181180
try {
182181
ArrowConverter::ToArrowSchema((ArrowSchema *)*out_schema, wrapper->result->types, wrapper->result->names,
183182
wrapper->result->client_properties);
184-
} catch (std::exception &ex) {
185-
wrapper->result->SetError(ErrorData(ex));
186-
return DuckDBError;
187183
} catch (...) {
188-
wrapper->result->SetError(ErrorData("Unknown error in duckdb_query_arrow_schema"));
189184
return DuckDBError;
190185
}
191186
return DuckDBSuccess;

test/api/capi/test_capi_geometry.cpp

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#include "capi_tester.hpp"
2-
#include "duckdb/common/arrow/schema_metadata.hpp"
32

43
using namespace duckdb;
54
using namespace std;
@@ -53,45 +52,3 @@ TEST_CASE("Test C API GEOMETRY type support", "[capi]") {
5352
REQUIRE(x == 42);
5453
REQUIRE(y == 1337);
5554
}
56-
57-
TEST_CASE("Test C API GEOMETRY arrow schema export with CRS", "[capi][arrow]") {
58-
// Regression test: exporting a GEOMETRY column with a CRS to an Arrow schema
59-
// used to assert in TransactionContext::ActiveTransaction(), because
60-
// ArrowGeometry::PopulateSchema resolves the CRS via the catalog and the
61-
// auto-commit transaction from the previous statement has already closed by
62-
// the time duckdb_query_arrow_schema runs.
63-
CAPITester tester;
64-
REQUIRE(tester.OpenDatabase(nullptr));
65-
REQUIRE_NO_FAIL(tester.Query("CREATE TABLE t1 (data GEOMETRY('OGC:CRS84'))"));
66-
REQUIRE_NO_FAIL(tester.Query("INSERT INTO t1 VALUES ('POINT(42 1337)')"));
67-
68-
duckdb_arrow arrow_result = nullptr;
69-
auto state = duckdb_query_arrow(tester.connection, "SELECT data FROM t1", &arrow_result);
70-
REQUIRE(state == DuckDBSuccess);
71-
72-
ArrowSchema arrow_schema;
73-
arrow_schema.Init();
74-
auto arrow_schema_ptr = &arrow_schema;
75-
state = duckdb_query_arrow_schema(arrow_result, reinterpret_cast<duckdb_arrow_schema *>(&arrow_schema_ptr));
76-
std::string err;
77-
if (state != DuckDBSuccess) {
78-
auto err_cstr = duckdb_query_arrow_error(arrow_result);
79-
err = err_cstr ? err_cstr : "(no error message)";
80-
duckdb_destroy_arrow(&arrow_result);
81-
}
82-
INFO("duckdb_query_arrow_schema error: " << err);
83-
REQUIRE(state == DuckDBSuccess);
84-
REQUIRE(arrow_schema.n_children == 1);
85-
86-
// The geoarrow.wkb extension name should appear in the child schema metadata.
87-
auto child = arrow_schema.children[0];
88-
REQUIRE(child != nullptr);
89-
REQUIRE(child->metadata != nullptr);
90-
auto md = ArrowSchemaMetadata(child->metadata);
91-
REQUIRE(md.GetOption(ArrowSchemaMetadata::ARROW_EXTENSION_NAME) == "geoarrow.wkb");
92-
93-
if (arrow_schema.release) {
94-
arrow_schema.release(arrow_schema_ptr);
95-
}
96-
duckdb_destroy_arrow(&arrow_result);
97-
}

0 commit comments

Comments
 (0)