From 4a7354e5f8cdd56b3a28fad1b860bb7893b20c3d Mon Sep 17 00:00:00 2001 From: Jim Castillo Date: Tue, 20 Jan 2026 13:09:16 -0800 Subject: [PATCH 1/2] Refactor to use new duckdb-go-api helper functions Use SetBindError, SetInitError, SetFunctionError helpers for error handling with automatic memory management, reducing boilerplate from 3-4 lines to 1. Use AssignStringToVector and AssignBytesToVector helpers for string/binary assignment, simplifying type conversion code. Changes: - table_function.go: Replace ~8 error blocks, update string/binary conversions - config_function.go: Use AssignStringToVector for result message - version_function.go: Use AssignStringToVector for version output - Update duckdb-go-api submodule with new helper functions - Fix test credentials (gizmosql_user) and TLS settings --- config_function.go | 5 +- duckarrow-local.sql | 2 +- duckdb-go-api | 2 +- table_function.go | 93 ++++++--------------------------- tests/connection_pool_test.sql | 4 +- tests/connection_test.sql | 2 +- tests/data_types_test.sql | 2 +- tests/error_handling_test.sql | 10 ++-- tests/full_test.sql | 2 +- tests/integration_test.sql | 2 +- tests/replacement_scan_test.sql | 4 +- version_function.go | 5 +- 12 files changed, 33 insertions(+), 100 deletions(-) diff --git a/config_function.go b/config_function.go index 6581938..4a6ef00 100644 --- a/config_function.go +++ b/config_function.go @@ -150,10 +150,7 @@ func duckarrow_configure_callback(info C.duckdb_function_info, input C.duckdb_da SetDuckArrowConfig(uri, username, password, skipVerify) // Return a confirmation message - result := "DuckArrow configured successfully" - resultCStr := C.CString(result) - C.duckdb_vector_assign_string_element(output, i, resultCStr) - C.free(unsafe.Pointer(resultCStr)) + duckdb.AssignStringToVector(duckdb.Vector{Ptr: unsafe.Pointer(output)}, int(i), "DuckArrow configured successfully") } } diff --git a/duckarrow-local.sql b/duckarrow-local.sql index eb9c313..d4bedb2 100644 --- a/duckarrow-local.sql +++ b/duckarrow-local.sql @@ -6,7 +6,7 @@ LOAD './build/duckarrow.duckdb_extension'; -- Configure duckarrow with credentials (enables replacement scan) -SELECT duckarrow_configure('grpc+tls://localhost:31337', 'duckarrow_user', 'duckarrow_password'); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password'); -- Query remote tables directly using duckarrow."TableName" syntax: -- SELECT * FROM duckarrow."Order" LIMIT 5; diff --git a/duckdb-go-api b/duckdb-go-api index 5f21830..0de6cda 160000 --- a/duckdb-go-api +++ b/duckdb-go-api @@ -1 +1 @@ -Subproject commit 5f218304bcbea542a4f13b788b11e555b0c9afe8 +Subproject commit 0de6cda6a9a01e1ec1a85ea3af3400df01f962c6 diff --git a/table_function.go b/table_function.go index 97dbd33..34b19ee 100644 --- a/table_function.go +++ b/table_function.go @@ -104,9 +104,7 @@ func duckarrow_bind_wrapper(info C.duckdb_bind_info) { ctx := context.Background() connResult, err := flight.GetConnection(ctx, cfg) if err != nil { - errStr := C.CString(fmt.Sprintf("connection failed: %v", err)) - C.duckdb_bind_set_error(info, errStr) - C.free(unsafe.Pointer(errStr)) + duckdb.SetBindError(duckdb.BindInfo{Ptr: unsafe.Pointer(info)}, "connection failed: %v", err) return } @@ -130,9 +128,7 @@ func duckarrow_bind_wrapper(info C.duckdb_bind_info) { } else { connResult.Client.Close() } - errStr := C.CString(fmt.Sprintf("query failed: %v", err)) - C.duckdb_bind_set_error(info, errStr) - C.free(unsafe.Pointer(errStr)) + duckdb.SetBindError(duckdb.BindInfo{Ptr: unsafe.Pointer(info)}, "query failed: %v", err) return } @@ -334,9 +330,7 @@ func duckarrow_init_wrapper(info C.duckdb_init_info) { for i := C.idx_t(0); i < columnCount; i++ { colIdx := C.duckdb_init_get_column_index(info, i) if int(colIdx) >= len(bindData.AllColumns) { - errStr := C.CString(fmt.Sprintf("column index %d out of range (max %d)", colIdx, len(bindData.AllColumns)-1)) - C.duckdb_init_set_error(info, errStr) - C.free(unsafe.Pointer(errStr)) + duckdb.SetInitError(duckdb.InitInfo{Ptr: unsafe.Pointer(info)}, "column index %d out of range (max %d)", colIdx, len(bindData.AllColumns)-1) return } projectedColumns[i] = bindData.AllColumns[colIdx] @@ -349,9 +343,7 @@ func duckarrow_init_wrapper(info C.duckdb_init_info) { ctx := context.Background() result, err := bindData.Client.Query(ctx, query) if err != nil { - errStr := C.CString(fmt.Sprintf("query execution failed: %v", err)) - C.duckdb_init_set_error(info, errStr) - C.free(unsafe.Pointer(errStr)) + duckdb.SetInitError(duckdb.InitInfo{Ptr: unsafe.Pointer(info)}, "query execution failed: %v", err) return } @@ -370,9 +362,7 @@ func duckarrow_scan_wrapper(info C.duckdb_function_info, output C.duckdb_data_ch bindHandle := cgo.Handle(uintptr(bindPtr)) bindData, ok := bindHandle.Value().(*BindData) if !ok { - errStr := C.CString("internal error: invalid bind data type") - C.duckdb_function_set_error(info, errStr) - C.free(unsafe.Pointer(errStr)) + duckdb.SetFunctionError(duckdb.FunctionInfo{Ptr: unsafe.Pointer(info)}, "internal error: invalid bind data type") return } @@ -381,9 +371,7 @@ func duckarrow_scan_wrapper(info C.duckdb_function_info, output C.duckdb_data_ch stateHandle := cgo.Handle(uintptr(statePtr)) state, ok := stateHandle.Value().(*ScanState) if !ok { - errStr := C.CString("internal error: invalid scan state type") - C.duckdb_function_set_error(info, errStr) - C.free(unsafe.Pointer(errStr)) + duckdb.SetFunctionError(duckdb.FunctionInfo{Ptr: unsafe.Pointer(info)}, "internal error: invalid scan state type") return } @@ -414,13 +402,8 @@ func scanHardcodedData(info C.duckdb_function_info, output C.duckdb_data_chunk, nameVec := duckdb.DataChunkGetVector(duckdb.DataChunk{Ptr: unsafe.Pointer(output)}, 1) for i, row := range rows { - idStr := C.CString(row.id) - C.duckdb_vector_assign_string_element(C.duckdb_vector(idVec.Ptr), C.idx_t(i), idStr) - C.free(unsafe.Pointer(idStr)) - - nameStr := C.CString(row.name) - C.duckdb_vector_assign_string_element(C.duckdb_vector(nameVec.Ptr), C.idx_t(i), nameStr) - C.free(unsafe.Pointer(nameStr)) + duckdb.AssignStringToVector(idVec, i, row.id) + duckdb.AssignStringToVector(nameVec, i, row.name) } C.duckdb_data_chunk_set_size(output, C.idx_t(len(rows))) @@ -444,9 +427,7 @@ func scanArrowData(info C.duckdb_function_info, output C.duckdb_data_chunk, bind if !bindData.Reader.Next() { if err := bindData.Reader.Err(); err != nil { - errStr := C.CString(err.Error()) - C.duckdb_function_set_error(info, errStr) - C.free(unsafe.Pointer(errStr)) + duckdb.SetFunctionError(duckdb.FunctionInfo{Ptr: unsafe.Pointer(info)}, "%v", err) } atomic.StoreInt32(&state.Done, 1) C.duckdb_data_chunk_set_size(output, 0) @@ -474,9 +455,7 @@ func scanArrowData(info C.duckdb_function_info, output C.duckdb_data_chunk, bind duckVec := duckdb.DataChunkGetVector(duckdb.DataChunk{Ptr: unsafe.Pointer(output)}, uint64(colIdx)) if err := convertArrowToDuckDB(arrowCol, duckVec, int(state.BatchPosition), rowsToEmit); err != nil { - errStr := C.CString(fmt.Sprintf("convert col %d: %v", colIdx, err)) - C.duckdb_function_set_error(info, errStr) - C.free(unsafe.Pointer(errStr)) + duckdb.SetFunctionError(duckdb.FunctionInfo{Ptr: unsafe.Pointer(info)}, "convert col %d: %v", colIdx, err) return } } @@ -500,9 +479,7 @@ func convertArrowToDuckDB(arrowCol arrow.Array, duckVec duckdb.Vector, offset, c duckdb.ValiditySetRowInvalid(validity, uint64(i)) continue } - cStr := C.CString(col.Value(srcIdx)) - C.duckdb_vector_assign_string_element(C.duckdb_vector(duckVec.Ptr), C.idx_t(i), cStr) - C.free(unsafe.Pointer(cStr)) + duckdb.AssignStringToVector(duckVec, i, col.Value(srcIdx)) } case *array.LargeString: @@ -512,9 +489,7 @@ func convertArrowToDuckDB(arrowCol arrow.Array, duckVec duckdb.Vector, offset, c duckdb.ValiditySetRowInvalid(validity, uint64(i)) continue } - cStr := C.CString(col.Value(srcIdx)) - C.duckdb_vector_assign_string_element(C.duckdb_vector(duckVec.Ptr), C.idx_t(i), cStr) - C.free(unsafe.Pointer(cStr)) + duckdb.AssignStringToVector(duckVec, i, col.Value(srcIdx)) } case *array.Int64: @@ -763,23 +738,7 @@ func convertArrowToDuckDB(arrowCol arrow.Array, duckVec duckdb.Vector, offset, c duckdb.ValiditySetRowInvalid(validity, uint64(i)) continue } - data := col.Value(srcIdx) - if len(data) == 0 { - // Empty binary - assign empty string - C.duckdb_vector_assign_string_element_len( - C.duckdb_vector(duckVec.Ptr), - C.idx_t(i), - nil, - 0, - ) - continue - } - C.duckdb_vector_assign_string_element_len( - C.duckdb_vector(duckVec.Ptr), - C.idx_t(i), - (*C.char)(unsafe.Pointer(&data[0])), - C.idx_t(len(data)), - ) + duckdb.AssignBytesToVector(duckVec, i, col.Value(srcIdx)) } case *array.LargeBinary: @@ -790,23 +749,7 @@ func convertArrowToDuckDB(arrowCol arrow.Array, duckVec duckdb.Vector, offset, c duckdb.ValiditySetRowInvalid(validity, uint64(i)) continue } - data := col.Value(srcIdx) - if len(data) == 0 { - // Empty binary - assign empty string - C.duckdb_vector_assign_string_element_len( - C.duckdb_vector(duckVec.Ptr), - C.idx_t(i), - nil, - 0, - ) - continue - } - C.duckdb_vector_assign_string_element_len( - C.duckdb_vector(duckVec.Ptr), - C.idx_t(i), - (*C.char)(unsafe.Pointer(&data[0])), - C.idx_t(len(data)), - ) + duckdb.AssignBytesToVector(duckVec, i, col.Value(srcIdx)) } case *array.FixedSizeBinary: @@ -826,9 +769,7 @@ func convertArrowToDuckDB(arrowCol arrow.Array, duckVec duckdb.Vector, offset, c } else { val = fmt.Sprintf("%x", data) } - cStr := C.CString(val) - C.duckdb_vector_assign_string_element(C.duckdb_vector(duckVec.Ptr), C.idx_t(i), cStr) - C.free(unsafe.Pointer(cStr)) + duckdb.AssignStringToVector(duckVec, i, val) } case *array.Decimal128: @@ -1162,9 +1103,7 @@ func convertArrowToDuckDB(arrowCol arrow.Array, duckVec duckdb.Vector, offset, c val = fmt.Sprintf("%v", arrowCol.GetOneForMarshal(srcIdx)) } - cStr := C.CString(val) - C.duckdb_vector_assign_string_element(C.duckdb_vector(duckVec.Ptr), C.idx_t(i), cStr) - C.free(unsafe.Pointer(cStr)) + duckdb.AssignStringToVector(duckVec, i, val) } } diff --git a/tests/connection_pool_test.sql b/tests/connection_pool_test.sql index c4c7716..b5c5865 100644 --- a/tests/connection_pool_test.sql +++ b/tests/connection_pool_test.sql @@ -8,7 +8,7 @@ LOAD './build/duckarrow.duckdb_extension'; -- Configure SELECT '=== Configure ===' as test; -SELECT duckarrow_configure('grpc+tls://localhost:31337', 'duckarrow_user', 'duckarrow_password', true); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); -- First query (cold connection - expect ~150-200ms) SELECT '=== Query 1 (cold connection) ===' as test; @@ -32,7 +32,7 @@ SELECT id, name FROM duckarrow."Order" LIMIT 3; -- Test reconfiguration creates new connection (same credentials, new pool entry) SELECT '=== Reconfigure (same server, fresh connection) ===' as test; -SELECT duckarrow_configure('grpc+tls://localhost:31337', 'duckarrow_user', 'duckarrow_password', true); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); -- First query after reconfig (should use existing pool) SELECT '=== Query 6 (after reconfig) ===' as test; diff --git a/tests/connection_test.sql b/tests/connection_test.sql index e6e775a..9258d0e 100644 --- a/tests/connection_test.sql +++ b/tests/connection_test.sql @@ -1,7 +1,7 @@ .bail on LOAD './build/duckarrow.duckdb_extension'; -- Configure credentials -SELECT duckarrow_configure('grpc+tls://localhost:31337', 'duckarrow_user', 'duckarrow_password', true); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); SELECT * FROM duckarrow_query( 'grpc+tls://localhost:31337', 'SELECT 1 as test_value' diff --git a/tests/data_types_test.sql b/tests/data_types_test.sql index 363ad51..696078d 100644 --- a/tests/data_types_test.sql +++ b/tests/data_types_test.sql @@ -6,7 +6,7 @@ -- Load extension and configure LOAD './build/duckarrow.duckdb_extension'; -SELECT duckarrow_configure('grpc://localhost:31337', 'duckarrow_user', 'duckarrow_password'); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); .print '=== Data Type Tests ===' diff --git a/tests/error_handling_test.sql b/tests/error_handling_test.sql index 92b3a1b..9e532cb 100644 --- a/tests/error_handling_test.sql +++ b/tests/error_handling_test.sql @@ -16,7 +16,7 @@ LOAD './build/duckarrow.duckdb_extension'; .bail off -- First configure correctly -SELECT duckarrow_configure('grpc://localhost:31337', 'duckarrow_user', 'duckarrow_password'); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); -- This should fail with a syntax error from the remote server SELECT * FROM duckarrow."Order" WHERE SELECTT; @@ -30,7 +30,7 @@ SELECT * FROM duckarrow."Order" WHERE SELECTT; .print '--- Test 2: Non-existent Table ---' -- Reconfigure (in case previous test affected state) -SELECT duckarrow_configure('grpc://localhost:31337', 'duckarrow_user', 'duckarrow_password'); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); -- This should fail with table not found SELECT * FROM duckarrow."ThisTableDefinitelyDoesNotExist12345"; @@ -94,7 +94,7 @@ SELECT 1 FROM duckarrow."Order" LIMIT 1; .print '--- Test 7: Invalid Credentials ---' -- Configure with wrong credentials -SELECT duckarrow_configure('grpc://localhost:31337', 'wrong_user', 'wrong_pass'); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'wrong_user', 'wrong_pass'); -- This should fail with auth error SELECT * FROM duckarrow."Order" LIMIT 1; @@ -108,7 +108,7 @@ SELECT * FROM duckarrow."Order" LIMIT 1; .print '--- Test 8: SQL Injection Prevention ---' -- Restore valid config first -SELECT duckarrow_configure('grpc://localhost:31337', 'duckarrow_user', 'duckarrow_password'); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); -- These should be blocked by table name validation -- Semicolon injection attempt @@ -151,7 +151,7 @@ SELECT * FROM duckarrow."malicious*/Order"; .bail on -- Restore valid configuration -SELECT duckarrow_configure('grpc://localhost:31337', 'duckarrow_user', 'duckarrow_password'); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); -- This should work after all the error tests SELECT COUNT(*) as recovery_test FROM duckarrow."Order"; diff --git a/tests/full_test.sql b/tests/full_test.sql index 7f26e40..7960a14 100644 --- a/tests/full_test.sql +++ b/tests/full_test.sql @@ -4,7 +4,7 @@ LOAD './build/duckarrow.duckdb_extension'; -- Configure credentials -SELECT duckarrow_configure('grpc+tls://localhost:31337', 'duckarrow_user', 'duckarrow_password', true); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); -- Test 1: Basic query with real data and types SELECT '=== Test 1: Basic query with types ===' as test; diff --git a/tests/integration_test.sql b/tests/integration_test.sql index 9ab103c..0cfcf51 100644 --- a/tests/integration_test.sql +++ b/tests/integration_test.sql @@ -6,7 +6,7 @@ -- Load extension and configure LOAD './build/duckarrow.duckdb_extension'; -SELECT duckarrow_configure('grpc://localhost:31337', 'duckarrow_user', 'duckarrow_password'); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); .print '=== Edge Case Integration Tests ===' diff --git a/tests/replacement_scan_test.sql b/tests/replacement_scan_test.sql index 4e6268a..7f32ce9 100644 --- a/tests/replacement_scan_test.sql +++ b/tests/replacement_scan_test.sql @@ -5,7 +5,7 @@ LOAD './build/duckarrow.duckdb_extension'; -- Test 1: Configure duckarrow URI SELECT '=== Test 1: Configure duckarrow URI ===' as test; -SELECT duckarrow_configure('grpc+tls://localhost:31337', 'duckarrow_user', 'duckarrow_password', true); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); -- Test 2: Basic replacement scan query SELECT '=== Test 2: Basic replacement scan ===' as test; @@ -73,7 +73,7 @@ SELECT duckarrow_configure('grpc://localhost:1234', 'user', 'pass'); -- Test 16: Reconfigure back to TLS for remaining tests SELECT '=== Test 16: Reconfigure to TLS ===' as test; -SELECT duckarrow_configure('grpc+tls://localhost:31337', 'duckarrow_user', 'duckarrow_password', true); +SELECT duckarrow_configure('grpc+tls://localhost:31337', 'gizmosql_user', 'gizmosql_password', true); -- Test 17: NULL handling SELECT '=== Test 17: NULL handling ===' as test; diff --git a/version_function.go b/version_function.go index e28a084..f0c645e 100644 --- a/version_function.go +++ b/version_function.go @@ -35,11 +35,8 @@ func duckarrow_version_callback(info C.duckdb_function_info, input C.duckdb_data } // Return the version string for each row - versionCStr := C.CString(Version) - defer C.free(unsafe.Pointer(versionCStr)) - for i := range inputSize { - C.duckdb_vector_assign_string_element(output, C.idx_t(i), versionCStr) + duckdb.AssignStringToVector(duckdb.Vector{Ptr: unsafe.Pointer(output)}, i, Version) } } From bbe50db045f72f1caab66c90e387fb5307a5ad2e Mon Sep 17 00:00:00 2001 From: Jim Castillo Date: Tue, 20 Jan 2026 13:34:28 -0800 Subject: [PATCH 2/2] Fix: Embed Extension API v1.2.0 in metadata DuckDB CLI v1.4.0 only supports loading extensions built for C API v1.2.0. Update Makefile to embed v1.2.0 (matching main.go's duckdb.Init call) so the extension can load correctly. The headers from v1.4.0 are still used for new features, but the declared API version must be v1.2.0 for compatibility. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index bf1d73f..555ae41 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -DUCKDB_VERSION := v1.4.0 +DUCKDB_VERSION := v1.2.0 EXTENSION_NAME := duckarrow # Extract version from git tag, fall back to "dev" for local builds EXTENSION_VERSION := $(shell git describe --tags --exact-match 2>/dev/null || echo "dev")