You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ze...@apache.org on 2023/01/24 20:32:39 UTC
[arrow-adbc] branch main updated: ci(c/driver/flightsql): Add SQLite flightsql server test (#378)
This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new a2178c6 ci(c/driver/flightsql): Add SQLite flightsql server test (#378)
a2178c6 is described below
commit a2178c6d0559043e09f9231a22e012f92df4064a
Author: Matt Topol <zo...@gmail.com>
AuthorDate: Tue Jan 24 15:32:32 2023 -0500
ci(c/driver/flightsql): Add SQLite flightsql server test (#378)
* ci(c/driver/flightsql): Add more tests for SQLite FlightSQL
* add docker compose definition and GHA runs
* fix upgrade to arrow master
* use 0.0.0.0 to bind
* put the port in docker-compose
* pre-commit linting
* helps if i lint with the right version of clang-format
* Update .github/workflows/native-unix.yml
Co-authored-by: Sutou Kouhei <ko...@cozmixng.org>
* Update .github/workflows/native-unix.yml
Co-authored-by: Sutou Kouhei <ko...@cozmixng.org>
* use Go asan when compiling with ADBC_USE_ASAN
* Fix titles to match other usage
* solve ASAN issue and ensure we don't have Go side leaks in unit tests
* pre-commit can jump in a lake, oy.
* add comments for why we call runtime.GC
* clang-format is very specific
Co-authored-by: Sutou Kouhei <ko...@cozmixng.org>
---
.env | 2 +
.github/workflows/native-unix.yml | 11 ++-
c/cmake_modules/GoUtils.cmake | 16 ++--
c/driver/flightsql/CMakeLists.txt | 1 +
c/driver/flightsql/dremio_flightsql_test.cc | 20 ++---
..._flightsql_test.cc => sqlite_flightsql_test.cc} | 32 ++++----
c/validation/adbc_validation.cc | 63 ++++++++++------
.../docker/golang-flightsql-sqlite.dockerfile | 23 ++----
docker-compose.yml | 13 ++++
go/adbc/driver/flightsql/flightsql_adbc_test.go | 12 +--
go/adbc/driver/flightsql/record_reader.go | 1 +
go/adbc/go.mod | 2 +-
go/adbc/go.sum | 6 +-
go/adbc/pkg/_tmpl/driver.go.tmpl | 23 +++++-
go/adbc/pkg/flightsql/driver.go | 25 ++++++-
go/adbc/standard_schemas.go | 2 +-
go/adbc/validation/validation.go | 85 +++++++++++++++++++++-
17 files changed, 249 insertions(+), 88 deletions(-)
diff --git a/.env b/.env
index 2fe295e..9a9d958 100644
--- a/.env
+++ b/.env
@@ -31,6 +31,8 @@ JDK=8
MANYLINUX=2014
MAVEN=3.5.4
PYTHON=3.10
+GO=1.19.5
+ARROW_MAJOR_VERSION=11
# Used through docker-compose.yml and serves as the default version for the
# ci/scripts/install_vcpkg.sh script.
diff --git a/.github/workflows/native-unix.yml b/.github/workflows/native-unix.yml
index 7f96ec3..35b480e 100644
--- a/.github/workflows/native-unix.yml
+++ b/.github/workflows/native-unix.yml
@@ -290,7 +290,11 @@ jobs:
BUILD_DRIVER_FLIGHTSQL: "1"
run: |
./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
- - name: Test FlightSQL Driver against Dremio
+ - name: Start SQLite server
+ shell: bash -l {0}
+ run: |
+ docker-compose up -d golang-sqlite-flightsql
+ - name: Test FlightSQL Driver against Dremio and SQLite
shell: bash -l {0}
env:
BUILD_ALL: "0"
@@ -298,9 +302,14 @@ jobs:
ADBC_DREMIO_FLIGHTSQL_URI: "grpc+tcp://localhost:32010"
ADBC_DREMIO_FLIGHTSQL_USER: "dremio"
ADBC_DREMIO_FLIGHTSQL_PASS: "dremio123"
+ ADBC_SQLITE_FLIGHTSQL_URI: "grpc+tcp://localhost:8080"
run: |
./ci/scripts/cpp_build.sh "$(pwd)" "$(pwd)/build"
./ci/scripts/cpp_test.sh "$(pwd)" "$(pwd)/build"
+ - name: Stop SQLite server
+ shell: bash -l {0}
+ run: |
+ docker-compose down
# ------------------------------------------------------------
# GLib/Ruby
diff --git a/c/cmake_modules/GoUtils.cmake b/c/cmake_modules/GoUtils.cmake
index 7b6e65a..d936fdf 100644
--- a/c/cmake_modules/GoUtils.cmake
+++ b/c/cmake_modules/GoUtils.cmake
@@ -61,6 +61,15 @@ function(add_go_lib GO_MOD_DIR GO_LIBNAME)
list(TRANSFORM ARG_SOURCES PREPEND "${GO_MOD_DIR}/")
+ # go asan only works on linux/amd64 and linux/arm64
+ if(ADBC_USE_ASAN)
+ if(CMAKE_SYSTEM_PROCESSOR STREQUAL "AMD64" OR CMAKE_SYSTEM_PROCESSOR STREQUAL "ARM64")
+ if(NOT APPLE AND NOT MSVC_TOOLCHAIN)
+ set(GO_BUILD_FLAGS "-asan")
+ endif()
+ endif()
+ endif()
+
if(BUILD_SHARED)
set(LIB_NAME_SHARED
"${CMAKE_SHARED_LIBRARY_PREFIX}${GO_LIBNAME}${CMAKE_SHARED_LIBRARY_SUFFIX}")
@@ -89,7 +98,7 @@ function(add_go_lib GO_MOD_DIR GO_LIBNAME)
WORKING_DIRECTORY ${GO_MOD_DIR}
DEPENDS ${ARG_SOURCES}
COMMAND ${CMAKE_COMMAND} -E env "${GO_ENV_VARS}" ${GO_BIN} build
- "${GO_BUILD_TAGS}" -o
+ "${GO_BUILD_TAGS}" "${GO_BUILD_FLAGS}" -o
"${LIBOUT_SHARED}.${ADBC_FULL_SO_VERSION}"
-buildmode=c-shared "${GO_LDFLAGS}" .
COMMAND ${CMAKE_COMMAND} -E remove -f
@@ -165,7 +174,7 @@ function(add_go_lib GO_MOD_DIR GO_LIBNAME)
WORKING_DIRECTORY ${GO_MOD_DIR}
DEPENDS ${ARG_SOURCES}
COMMAND ${GO_BIN} build "${GO_BUILD_TAGS}" -o "${LIBOUT_STATIC}"
- -buildmode=c-archive .
+ -buildmode=c-archive "${GO_BUILD_FLAGS}" .
COMMAND ${CMAKE_COMMAND} -E remove -f "${LIBOUT_HEADER}"
COMMENT "Building Go Static lib ${GO_LIBNAME}"
COMMAND_EXPAND_LISTS)
@@ -181,9 +190,6 @@ function(add_go_lib GO_MOD_DIR GO_LIBNAME)
install(FILES "${LIBOUT_STATIC}" TYPE LIB)
endif()
- # if(ARG_CMAKE_PACKAGE_NAME)
- # install_cmake_package(${ARG_CMAKE_PACKAGE_NAME} ${GO_LIBNAME}_export)
- # endif()
if(ARG_PKG_CONFIG_NAME)
arrow_add_pkg_config("${ARG_PKG_CONFIG_NAME}")
diff --git a/c/driver/flightsql/CMakeLists.txt b/c/driver/flightsql/CMakeLists.txt
index a450fd9..f882e3f 100644
--- a/c/driver/flightsql/CMakeLists.txt
+++ b/c/driver/flightsql/CMakeLists.txt
@@ -56,6 +56,7 @@ if(ADBC_BUILD_TESTS)
adbc
SOURCES
dremio_flightsql_test.cc
+ sqlite_flightsql_test.cc
../../validation/adbc_validation.cc
../../validation/adbc_validation_util.cc
EXTRA_LINK_LIBS
diff --git a/c/driver/flightsql/dremio_flightsql_test.cc b/c/driver/flightsql/dremio_flightsql_test.cc
index f9ee56d..b7c6330 100644
--- a/c/driver/flightsql/dremio_flightsql_test.cc
+++ b/c/driver/flightsql/dremio_flightsql_test.cc
@@ -26,7 +26,7 @@
using adbc_validation::IsOkStatus;
-class DremioFlightSQLQuirks : public adbc_validation::DriverQuirks {
+class DremioFlightSqlQuirks : public adbc_validation::DriverQuirks {
public:
AdbcStatusCode SetupDatabase(struct AdbcDatabase* database,
struct AdbcError* error) const override {
@@ -51,18 +51,18 @@ class DremioFlightSQLQuirks : public adbc_validation::DriverQuirks {
bool supports_dynamic_parameter_binding() const override { return false; }
};
-class DremioFlightSQLTest : public ::testing::Test, public adbc_validation::DatabaseTest {
+class DremioFlightSqlTest : public ::testing::Test, public adbc_validation::DatabaseTest {
public:
const adbc_validation::DriverQuirks* quirks() const override { return &quirks_; }
void SetUp() override { ASSERT_NO_FATAL_FAILURE(SetUpTest()); }
void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownTest()); }
protected:
- DremioFlightSQLQuirks quirks_;
+ DremioFlightSqlQuirks quirks_;
};
-ADBCV_TEST_DATABASE(DremioFlightSQLTest)
+ADBCV_TEST_DATABASE(DremioFlightSqlTest)
-class DremioFlightSQLConnectionTest : public ::testing::Test,
+class DremioFlightSqlConnectionTest : public ::testing::Test,
public adbc_validation::ConnectionTest {
public:
const adbc_validation::DriverQuirks* quirks() const override { return &quirks_; }
@@ -70,11 +70,11 @@ class DremioFlightSQLConnectionTest : public ::testing::Test,
void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownTest()); }
protected:
- DremioFlightSQLQuirks quirks_;
+ DremioFlightSqlQuirks quirks_;
};
-ADBCV_TEST_CONNECTION(DremioFlightSQLConnectionTest)
+ADBCV_TEST_CONNECTION(DremioFlightSqlConnectionTest)
-class DremioFlightSQLStatementTest : public ::testing::Test,
+class DremioFlightSqlStatementTest : public ::testing::Test,
public adbc_validation::StatementTest {
public:
const adbc_validation::DriverQuirks* quirks() const override { return &quirks_; }
@@ -82,6 +82,6 @@ class DremioFlightSQLStatementTest : public ::testing::Test,
void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownTest()); }
protected:
- DremioFlightSQLQuirks quirks_;
+ DremioFlightSqlQuirks quirks_;
};
-ADBCV_TEST_STATEMENT(DremioFlightSQLStatementTest)
+ADBCV_TEST_STATEMENT(DremioFlightSqlStatementTest)
diff --git a/c/driver/flightsql/dremio_flightsql_test.cc b/c/driver/flightsql/sqlite_flightsql_test.cc
similarity index 73%
copy from c/driver/flightsql/dremio_flightsql_test.cc
copy to c/driver/flightsql/sqlite_flightsql_test.cc
index f9ee56d..d6a18e0 100644
--- a/c/driver/flightsql/dremio_flightsql_test.cc
+++ b/c/driver/flightsql/sqlite_flightsql_test.cc
@@ -26,43 +26,37 @@
using adbc_validation::IsOkStatus;
-class DremioFlightSQLQuirks : public adbc_validation::DriverQuirks {
+class SqliteFlightSqlQuirks : public adbc_validation::DriverQuirks {
public:
AdbcStatusCode SetupDatabase(struct AdbcDatabase* database,
struct AdbcError* error) const override {
- const char* uri = std::getenv("ADBC_DREMIO_FLIGHTSQL_URI");
- const char* user = std::getenv("ADBC_DREMIO_FLIGHTSQL_USER");
- const char* pass = std::getenv("ADBC_DREMIO_FLIGHTSQL_PASS");
+ const char* uri = std::getenv("ADBC_SQLITE_FLIGHTSQL_URI");
EXPECT_THAT(AdbcDatabaseSetOption(database, "uri", uri, error), IsOkStatus(error));
- EXPECT_THAT(AdbcDatabaseSetOption(database, "username", user, error),
- IsOkStatus(error));
- EXPECT_THAT(AdbcDatabaseSetOption(database, "password", pass, error),
- IsOkStatus(error));
return ADBC_STATUS_OK;
}
std::string BindParameter(int index) const override { return "?"; }
bool supports_concurrent_statements() const override { return true; }
bool supports_transactions() const override { return false; }
- bool supports_get_sql_info() const override { return false; }
+ bool supports_get_sql_info() const override { return true; }
bool supports_get_objects() const override { return false; }
bool supports_bulk_ingest() const override { return false; }
bool supports_partitioned_data() const override { return true; }
- bool supports_dynamic_parameter_binding() const override { return false; }
+ bool supports_dynamic_parameter_binding() const override { return true; }
};
-class DremioFlightSQLTest : public ::testing::Test, public adbc_validation::DatabaseTest {
+class SqliteFlightSqlTest : public ::testing::Test, public adbc_validation::DatabaseTest {
public:
const adbc_validation::DriverQuirks* quirks() const override { return &quirks_; }
void SetUp() override { ASSERT_NO_FATAL_FAILURE(SetUpTest()); }
void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownTest()); }
protected:
- DremioFlightSQLQuirks quirks_;
+ SqliteFlightSqlQuirks quirks_;
};
-ADBCV_TEST_DATABASE(DremioFlightSQLTest)
+ADBCV_TEST_DATABASE(SqliteFlightSqlTest)
-class DremioFlightSQLConnectionTest : public ::testing::Test,
+class SqliteFlightSqlConnectionTest : public ::testing::Test,
public adbc_validation::ConnectionTest {
public:
const adbc_validation::DriverQuirks* quirks() const override { return &quirks_; }
@@ -70,11 +64,11 @@ class DremioFlightSQLConnectionTest : public ::testing::Test,
void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownTest()); }
protected:
- DremioFlightSQLQuirks quirks_;
+ SqliteFlightSqlQuirks quirks_;
};
-ADBCV_TEST_CONNECTION(DremioFlightSQLConnectionTest)
+ADBCV_TEST_CONNECTION(SqliteFlightSqlConnectionTest)
-class DremioFlightSQLStatementTest : public ::testing::Test,
+class SqliteFlightSqlStatementTest : public ::testing::Test,
public adbc_validation::StatementTest {
public:
const adbc_validation::DriverQuirks* quirks() const override { return &quirks_; }
@@ -82,6 +76,6 @@ class DremioFlightSQLStatementTest : public ::testing::Test,
void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownTest()); }
protected:
- DremioFlightSQLQuirks quirks_;
+ SqliteFlightSqlQuirks quirks_;
};
-ADBCV_TEST_STATEMENT(DremioFlightSQLStatementTest)
+ADBCV_TEST_STATEMENT(SqliteFlightSqlStatementTest)
diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc
index 605f0f1..bb83013 100644
--- a/c/validation/adbc_validation.cc
+++ b/c/validation/adbc_validation.cc
@@ -1131,8 +1131,10 @@ void StatementTest::TestSqlPartitionedInts() {
// Assume only 1 partition
ASSERT_EQ(1, partitions->num_partitions);
ASSERT_THAT(rows_affected, ::testing::AnyOf(::testing::Eq(1), ::testing::Eq(-1)));
- ASSERT_NE(nullptr, schema->release);
- ASSERT_EQ(1, schema->n_children);
+ // it's allowed for Executepartitions to return a nil schema if one is not available
+ if (schema->release != nullptr) {
+ ASSERT_EQ(1, schema->n_children);
+ }
Handle<struct AdbcConnection> connection2;
StreamReader reader;
@@ -1179,14 +1181,19 @@ void StatementTest::TestSqlPrepareGetParameterSchema() {
query += quirks()->BindParameter(0);
query += ", ";
query += quirks()->BindParameter(1);
+
ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, query.c_str(), &error),
IsOkStatus(&error));
ASSERT_THAT(AdbcStatementPrepare(&statement, &error), IsOkStatus(&error));
Handle<struct ArrowSchema> schema;
+ // if schema cannot be determined we should get NOT IMPLEMENTED returned
ASSERT_THAT(AdbcStatementGetParameterSchema(&statement, &schema.value, &error),
- IsOkStatus(&error));
- ASSERT_EQ(2, schema->n_children);
+ ::testing::AnyOf(IsOkStatus(&error),
+ IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error)));
+ if (schema->release != nullptr) {
+ ASSERT_EQ(2, schema->n_children);
+ }
// Can't assume anything about names or types here
}
@@ -1263,25 +1270,39 @@ void StatementTest::TestSqlPrepareSelectParams() {
ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
ASSERT_EQ(2, reader.schema->n_children);
- ASSERT_NO_FATAL_FAILURE(reader.Next());
- ASSERT_NE(nullptr, reader.array->release);
- ASSERT_EQ(3, reader.array->length);
- ASSERT_EQ(2, reader.array->n_children);
+ const std::vector<std::optional<int32_t>> expected_int32{42, -42, std::nullopt};
+ const std::vector<std::optional<int64_t>> expected_int64{42, -42, std::nullopt};
+ const std::vector<std::optional<std::string>> expected_string{"", std::nullopt, "bar"};
- switch (reader.fields[0].data_type) {
- case NANOARROW_TYPE_INT32:
- ASSERT_NO_FATAL_FAILURE(
- CompareArray<int32_t>(reader.array_view->children[0], {42, -42, std::nullopt}));
- break;
- case NANOARROW_TYPE_INT64:
- ASSERT_NO_FATAL_FAILURE(
- CompareArray<int64_t>(reader.array_view->children[0], {42, -42, std::nullopt}));
- break;
- default:
- FAIL() << "Unexpected data type: " << reader.fields[0].data_type;
+ int64_t nrows = 0;
+ while (nrows < 3) {
+ ASSERT_NO_FATAL_FAILURE(reader.Next());
+ ASSERT_NE(nullptr, reader.array->release);
+ ASSERT_EQ(2, reader.array->n_children);
+
+ auto start = nrows;
+ auto end = nrows + reader.array->length;
+
+ switch (reader.fields[0].data_type) {
+ case NANOARROW_TYPE_INT32:
+ ASSERT_NO_FATAL_FAILURE(CompareArray<int32_t>(
+ reader.array_view->children[0],
+ {expected_int32.begin() + start, expected_int32.begin() + end}));
+ break;
+ case NANOARROW_TYPE_INT64:
+ ASSERT_NO_FATAL_FAILURE(CompareArray<int64_t>(
+ reader.array_view->children[0],
+ {expected_int64.begin() + start, expected_int64.begin() + end}));
+ break;
+ default:
+ FAIL() << "Unexpected data type: " << reader.fields[0].data_type;
+ }
+ ASSERT_NO_FATAL_FAILURE(CompareArray<std::string>(
+ reader.array_view->children[1],
+ {expected_string.begin() + start, expected_string.begin() + end}));
+ nrows += reader.array->length;
}
- ASSERT_NO_FATAL_FAILURE(CompareArray<std::string>(reader.array_view->children[1],
- {"", std::nullopt, "bar"}));
+ ASSERT_EQ(3, nrows);
ASSERT_NO_FATAL_FAILURE(reader.Next());
ASSERT_EQ(nullptr, reader.array->release);
diff --git a/.env b/ci/docker/golang-flightsql-sqlite.dockerfile
similarity index 59%
copy from .env
copy to ci/docker/golang-flightsql-sqlite.dockerfile
index 2fe295e..4c230b7 100644
--- a/.env
+++ b/ci/docker/golang-flightsql-sqlite.dockerfile
@@ -15,23 +15,12 @@
# specific language governing permissions and limitations
# under the License.
-# All of the following environment variables are required to set default values
-# for the parameters in docker-compose.yml.
+ARG GO
+FROM golang:${GO}
-# Default repository to pull and push images from
-REPO=apache/arrow-dev
+ARG ARROW_MAJOR_VERSION
-# different architecture notations
-ARCH=amd64
-ARCH_ALIAS=x86_64
-ARCH_SHORT=amd64
+RUN go install github.com/apache/arrow/go/v${ARROW_MAJOR_VERSION}/arrow/flight/flightsql/example/cmd/sqlite_flightsql_server@latest
+EXPOSE 8080
-# Default versions for various dependencies
-JDK=8
-MANYLINUX=2014
-MAVEN=3.5.4
-PYTHON=3.10
-
-# Used through docker-compose.yml and serves as the default version for the
-# ci/scripts/install_vcpkg.sh script.
-VCPKG="2871ddd918cecb9cb642bcb9c56897f397283192"
+ENTRYPOINT /go/bin/sqlite_flightsql_server -host 0.0.0.0 -port 8080
diff --git a/docker-compose.yml b/docker-compose.yml
index 0785b2f..6c78ece 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -30,6 +30,19 @@ services:
command: |
/bin/bash -c 'git config --global --add safe.directory /adbc && source /opt/conda/etc/profile.d/conda.sh && mamba create -y -n adbc -c conda-forge go --file /adbc/ci/conda_env_cpp.txt --file /adbc/ci/conda_env_docs.txt --file /adbc/ci/conda_env_python.txt && conda activate adbc && env ADBC_USE_ASAN=0 ADBC_USE_UBSAN=0 /adbc/ci/scripts/cpp_build.sh /adbc /adbc/build && env CGO_ENABLED=1 /adbc/ci/scripts/go_build.sh /adbc /adbc/build && /adbc/ci/scripts/python_build.sh /adbc /adbc/bui [...]
+ golang-sqlite-flightsql:
+ image: ${REPO}:golang-${GO}-sqlite-flightsql
+ build:
+ context: .
+ cache_from:
+ - ${REPO}:golang-${GO}-sqlite-flightsql
+ dockerfile: ci/docker/golang-flightsql-sqlite.dockerfile
+ args:
+ GO: ${GO}
+ ARROW_MAJOR_VERSION: ${ARROW_MAJOR_VERSION}
+ ports:
+ - 8080:8080
+
############################ Java JARs ######################################
java-dist:
diff --git a/go/adbc/driver/flightsql/flightsql_adbc_test.go b/go/adbc/driver/flightsql/flightsql_adbc_test.go
index 2f112d7..bdbfacc 100644
--- a/go/adbc/driver/flightsql/flightsql_adbc_test.go
+++ b/go/adbc/driver/flightsql/flightsql_adbc_test.go
@@ -170,11 +170,13 @@ func (s *FlightSQLQuirks) CreateSampleTable(tableName string, r arrow.Record) er
return nil
}
-func (s *FlightSQLQuirks) BindParameter(_ int) string { return "?" }
-func (s *FlightSQLQuirks) SupportsConcurrentStatements() bool { return true }
-func (s *FlightSQLQuirks) SupportsPartitionedData() bool { return true }
-func (s *FlightSQLQuirks) SupportsTransactions() bool { return false }
-func (s *FlightSQLQuirks) SupportsGetParameterSchema() bool { return false }
+func (s *FlightSQLQuirks) Alloc() memory.Allocator { return s.mem }
+func (s *FlightSQLQuirks) BindParameter(_ int) string { return "?" }
+func (s *FlightSQLQuirks) SupportsConcurrentStatements() bool { return true }
+func (s *FlightSQLQuirks) SupportsPartitionedData() bool { return true }
+func (s *FlightSQLQuirks) SupportsTransactions() bool { return false }
+func (s *FlightSQLQuirks) SupportsGetParameterSchema() bool { return false }
+func (s *FlightSQLQuirks) SupportsDynamicParameterBinding() bool { return true }
func (s *FlightSQLQuirks) GetMetadata(code adbc.InfoCode) interface{} {
switch code {
case adbc.InfoDriverName:
diff --git a/go/adbc/driver/flightsql/record_reader.go b/go/adbc/driver/flightsql/record_reader.go
index 4f99ba4..0b41fd3 100644
--- a/go/adbc/driver/flightsql/record_reader.go
+++ b/go/adbc/driver/flightsql/record_reader.go
@@ -71,6 +71,7 @@ func newRecordReader(ctx context.Context, alloc memory.Allocator, cl *flightsql.
} else {
rdr, err := doGet(ctx, cl, endpoints[0], clCache)
if err != nil {
+ close(ch)
return nil, adbcFromFlightStatus(err)
}
schema = rdr.Schema()
diff --git a/go/adbc/go.mod b/go/adbc/go.mod
index 094db7b..f6dc610 100644
--- a/go/adbc/go.mod
+++ b/go/adbc/go.mod
@@ -20,7 +20,7 @@ module github.com/apache/arrow-adbc/go/adbc
go 1.18
require (
- github.com/apache/arrow/go/v11 v11.0.0-20230120215317-f7aa50dbeccd
+ github.com/apache/arrow/go/v11 v11.0.0-20230123220137-8449c553710a
github.com/bluele/gcache v0.0.2
github.com/stretchr/testify v1.8.0
golang.org/x/exp v0.0.0-20220827204233-334a2380cb91
diff --git a/go/adbc/go.sum b/go/adbc/go.sum
index f462d2c..c62eb5d 100644
--- a/go/adbc/go.sum
+++ b/go/adbc/go.sum
@@ -3,10 +3,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03
github.com/JohnCGriffin/overflow v0.0.0-20211019200055-46fa312c352c h1:RGWPOewvKIROun94nF7v2cua9qP+thov/7M50KEoeSU=
github.com/andybalholm/brotli v1.0.4 h1:V7DdXeJtZscaqfNuAdSRuRFzuiKlHSC/Zh3zl9qY3JY=
github.com/andybalholm/brotli v1.0.4/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig=
-github.com/apache/arrow/go/v11 v11.0.0-20230119180814-bf8780d0ff79 h1:KGZHC3moCZssyuDkcY02ldssfWNMdbomjNYOCxKwY/Y=
-github.com/apache/arrow/go/v11 v11.0.0-20230119180814-bf8780d0ff79/go.mod h1:Eg5OsL5H+e299f7u5ssuXsuHQVEGC4xei5aX110hRiI=
-github.com/apache/arrow/go/v11 v11.0.0-20230120215317-f7aa50dbeccd h1:r8ofoR0ijMZ44ATCWSkkX7W/L7dn+oIe015O0x3VaEA=
-github.com/apache/arrow/go/v11 v11.0.0-20230120215317-f7aa50dbeccd/go.mod h1:Eg5OsL5H+e299f7u5ssuXsuHQVEGC4xei5aX110hRiI=
+github.com/apache/arrow/go/v11 v11.0.0-20230123220137-8449c553710a h1:46k0d+c2UMW3TBn5Q4x1rynQb7ISPQegpQCROt9w3Fw=
+github.com/apache/arrow/go/v11 v11.0.0-20230123220137-8449c553710a/go.mod h1:Eg5OsL5H+e299f7u5ssuXsuHQVEGC4xei5aX110hRiI=
github.com/apache/thrift v0.16.0 h1:qEy6UW60iVOlUy+b9ZR0d5WzUWYGOo4HfopoyBaNmoY=
github.com/apache/thrift v0.16.0/go.mod h1:PHK3hniurgQaNMZYaCLEqXKsYK8upmhPbmdP2FXSqgU=
github.com/bluele/gcache v0.0.2 h1:WcbfdXICg7G/DGBh1PFfcirkWOQV+v077yF1pSy3DGw=
diff --git a/go/adbc/pkg/_tmpl/driver.go.tmpl b/go/adbc/pkg/_tmpl/driver.go.tmpl
index c751edc..19101c5 100644
--- a/go/adbc/pkg/_tmpl/driver.go.tmpl
+++ b/go/adbc/pkg/_tmpl/driver.go.tmpl
@@ -34,6 +34,7 @@ import (
"context"
"errors"
"fmt"
+ "runtime"
"runtime/cgo"
"unsafe"
@@ -182,7 +183,13 @@ func {{.Prefix}}DatabaseRelease(db *C.struct_AdbcDatabase, err *C.struct_AdbcErr
C.free(unsafe.Pointer(db.private_data))
db.private_data = nil
h.Delete()
-
+ // manually trigger GC for two reasons:
+ // 1. ASAN expects the release callback to be called before
+ // the process ends, but GC is not deterministic. So by manually
+ // triggering the GC we ensure the release callback gets called.
+ // 2. Creates deterministic GC behavior by all Release functions
+ // triggering a garbage collection
+ runtime.GC()
return C.ADBC_STATUS_OK
}
@@ -275,6 +282,13 @@ func {{.Prefix}}ConnectionRelease(cnxn *C.struct_AdbcConnection, err *C.struct_A
C.free(unsafe.Pointer(cnxn.private_data))
cnxn.private_data = nil
h.Delete()
+ // manually trigger GC for two reasons:
+ // 1. ASAN expects the release callback to be called before
+ // the process ends, but GC is not deterministic. So by manually
+ // triggering the GC we ensure the release callback gets called.
+ // 2. Creates deterministic GC behavior by all Release functions
+ // triggering a garbage collection
+ runtime.GC()
}()
if conn.cnxn == nil {
return C.ADBC_STATUS_OK
@@ -475,6 +489,13 @@ func {{.Prefix}}StatementRelease(stmt *C.struct_AdbcStatement, err *C.struct_Adb
e := st.Close()
h.Delete()
+ // manually trigger GC for two reasons:
+ // 1. ASAN expects the release callback to be called before
+ // the process ends, but GC is not deterministic. So by manually
+ // triggering the GC we ensure the release callback gets called.
+ // 2. Creates deterministic GC behavior by all Release functions
+ // triggering a garbage collection
+ runtime.GC()
return C.AdbcStatusCode(errToAdbcErr(err, e))
}
diff --git a/go/adbc/pkg/flightsql/driver.go b/go/adbc/pkg/flightsql/driver.go
index 631b434..2fc916b 100644
--- a/go/adbc/pkg/flightsql/driver.go
+++ b/go/adbc/pkg/flightsql/driver.go
@@ -36,6 +36,7 @@ import (
"context"
"errors"
"fmt"
+ "runtime"
"runtime/cgo"
"unsafe"
@@ -178,13 +179,20 @@ func FlightSQLDatabaseRelease(db *C.struct_AdbcDatabase, err *C.struct_AdbcError
return C.ADBC_STATUS_INVALID_STATE
}
h := (*(*cgo.Handle)(db.private_data))
+
cdb := h.Value().(*cDatabase)
cdb.db = nil
cdb.opts = nil
C.free(unsafe.Pointer(db.private_data))
db.private_data = nil
h.Delete()
-
+ // manually trigger GC for two reasons:
+ // 1. ASAN expects the release callback to be called before
+ // the process ends, but GC is not deterministic. So by manually
+ // triggering the GC we ensure the release callback gets called.
+ // 2. Creates deterministic GC behavior by all Release functions
+ // triggering a garbage collection
+ runtime.GC()
return C.ADBC_STATUS_OK
}
@@ -277,11 +285,17 @@ func FlightSQLConnectionRelease(cnxn *C.struct_AdbcConnection, err *C.struct_Adb
C.free(unsafe.Pointer(cnxn.private_data))
cnxn.private_data = nil
h.Delete()
+ // manually trigger GC for two reasons:
+ // 1. ASAN expects the release callback to be called before
+ // the process ends, but GC is not deterministic. So by manually
+ // triggering the GC we ensure the release callback gets called.
+ // 2. Creates deterministic GC behavior by all Release functions
+ // triggering a garbage collection
+ runtime.GC()
}()
if conn.cnxn == nil {
return C.ADBC_STATUS_OK
}
-
return C.AdbcStatusCode(errToAdbcErr(err, conn.cnxn.Close()))
}
@@ -478,6 +492,13 @@ func FlightSQLStatementRelease(stmt *C.struct_AdbcStatement, err *C.struct_AdbcE
e := st.Close()
h.Delete()
+ // manually trigger GC for two reasons:
+ // 1. ASAN expects the release callback to be called before
+ // the process ends, but GC is not deterministic. So by manually
+ // triggering the GC we ensure the release callback gets called.
+ // 2. Creates deterministic GC behavior by all Release functions
+ // triggering a garbage collection
+ runtime.GC()
return C.AdbcStatusCode(errToAdbcErr(err, e))
}
diff --git a/go/adbc/standard_schemas.go b/go/adbc/standard_schemas.go
index 68fb3a7..303c7fb 100644
--- a/go/adbc/standard_schemas.go
+++ b/go/adbc/standard_schemas.go
@@ -34,7 +34,7 @@ var (
arrow.ListOf(arrow.PrimitiveTypes.Int32)), Nullable: true},
},
[]arrow.UnionTypeCode{0, 1, 2, 3, 4, 5},
- )},
+ ), Nullable: true},
}, nil)
TableTypesSchema = arrow.NewSchema([]arrow.Field{{Name: "table_type", Type: arrow.BinaryTypes.String}}, nil)
diff --git a/go/adbc/validation/validation.go b/go/adbc/validation/validation.go
index c94a3f6..da58544 100644
--- a/go/adbc/validation/validation.go
+++ b/go/adbc/validation/validation.go
@@ -52,10 +52,14 @@ type DriverQuirks interface {
SupportsTransactions() bool
// Whether retrieving the schema of prepared statement params is supported
SupportsGetParameterSchema() bool
+ // Whether it supports dynamic parameter binding in queries
+ SupportsDynamicParameterBinding() bool
// Expected Metadata responses
GetMetadata(adbc.InfoCode) interface{}
// Create a sample table from an arrow record
CreateSampleTable(tableName string, r arrow.Record) error
+
+ Alloc() memory.Allocator
}
type DatabaseTests struct {
@@ -216,7 +220,7 @@ func (c *ConnectionTests) TestMetadataGetInfo() {
}
func (c *ConnectionTests) TestMetadataGetTableSchema() {
- rec, _, err := array.RecordFromJSON(memory.DefaultAllocator, arrow.NewSchema(
+ rec, _, err := array.RecordFromJSON(c.Quirks.Alloc(), arrow.NewSchema(
[]arrow.Field{
{Name: "ints", Type: arrow.PrimitiveTypes.Int64, Nullable: true},
{Name: "strings", Type: arrow.BinaryTypes.String, Nullable: true},
@@ -387,6 +391,59 @@ func (s *StatementTests) TestSQLPrepareGetParameterSchema() {
}
}
+func (s *StatementTests) TestSQLPrepareSelectParams() {
+ if !s.Quirks.SupportsDynamicParameterBinding() {
+ s.T().SkipNow()
+ }
+
+ stmt, err := s.Cnxn.NewStatement()
+ s.NoError(err)
+ defer stmt.Close()
+
+ query := "SELECT " + s.Quirks.BindParameter(0) + ", " + s.Quirks.BindParameter(1)
+ s.Require().NoError(stmt.SetSqlQuery(query))
+ s.Require().NoError(stmt.Prepare(s.ctx))
+
+ schema := arrow.NewSchema([]arrow.Field{
+ {Name: "int64s", Type: arrow.PrimitiveTypes.Int64, Nullable: true},
+ {Name: "strings", Type: arrow.BinaryTypes.String, Nullable: true},
+ }, nil)
+
+ bldr := array.NewRecordBuilder(s.Quirks.Alloc(), schema)
+ defer bldr.Release()
+ bldr.Field(0).(*array.Int64Builder).AppendValues([]int64{42, -42, 0}, []bool{true, true, false})
+ bldr.Field(1).(*array.StringBuilder).AppendValues([]string{"", "", "bar"}, []bool{true, false, true})
+ batch := bldr.NewRecord()
+ defer batch.Release()
+
+ s.Require().NoError(stmt.Bind(s.ctx, batch))
+ rdr, affected, err := stmt.ExecuteQuery(s.ctx)
+ s.Require().NoError(err)
+ defer rdr.Release()
+ s.True(affected == 1 || affected == -1, affected)
+
+ var nrows int64
+ for rdr.Next() {
+ rec := rdr.Record()
+ s.Require().NotNil(rec)
+ s.EqualValues(2, rec.NumCols())
+
+ start, end := nrows, nrows+rec.NumRows()
+ switch arr := rec.Column(0).(type) {
+ case *array.Int32:
+
+ case *array.Int64:
+ s.True(array.SliceEqual(arr, 0, int64(arr.Len()), batch.Column(0), start, end))
+ }
+
+ s.True(array.SliceEqual(rec.Column(1), 0, rec.NumRows(), batch.Column(1), start, end))
+ nrows += rec.NumRows()
+ }
+ s.EqualValues(3, nrows)
+ s.False(rdr.Next())
+ s.NoError(rdr.Err())
+}
+
func (s *StatementTests) TestSQLPrepareSelectNoParams() {
stmt, err := s.Cnxn.NewStatement()
s.NoError(err)
@@ -418,3 +475,29 @@ func (s *StatementTests) TestSQLPrepareSelectNoParams() {
s.False(rdr.Next())
}
+
+func (s *StatementTests) TestSqlPrepareErrorParamCountMismatch() {
+ if !s.Quirks.SupportsDynamicParameterBinding() {
+ s.T().SkipNow()
+ }
+
+ query := "SELECT " + s.Quirks.BindParameter(0) + ", " + s.Quirks.BindParameter(1)
+ stmt, err := s.Cnxn.NewStatement()
+ s.NoError(err)
+ defer stmt.Close()
+
+ s.NoError(stmt.SetSqlQuery(query))
+ s.NoError(stmt.Prepare(s.ctx))
+
+ batchbldr := array.NewRecordBuilder(s.Quirks.Alloc(), arrow.NewSchema(
+ []arrow.Field{{Name: "int64s", Type: arrow.PrimitiveTypes.Int64}}, nil))
+ defer batchbldr.Release()
+ bldr := batchbldr.Field(0).(*array.Int64Builder)
+ bldr.AppendValues([]int64{42, -42, 0}, []bool{true, true, false})
+ batch := batchbldr.NewRecord()
+ defer batch.Release()
+
+ s.NoError(stmt.Bind(s.ctx, batch))
+ _, _, err = stmt.ExecuteQuery(s.ctx)
+ s.Error(err)
+}