This is an automated email from the ASF dual-hosted git repository.
lidavidm 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 63bb903b7 test(c): don't use sketchy cast to test backwards
compatibility (#2425)
63bb903b7 is described below
commit 63bb903b7ddc7730beaa3d092dc5808632cd4b08
Author: David Li <[email protected]>
AuthorDate: Thu Jan 9 01:14:05 2025 -0500
test(c): don't use sketchy cast to test backwards compatibility (#2425)
- Backport nanoarrow patch to satisfy newer Clang
- Add test using GCC 15
- Update tests using sketchy casts to satisfy these compilers
- Refactor the clang/gcc Docker jobs
Fixes #2424.
---------
Co-authored-by: Sutou Kouhei <[email protected]>
---
.github/workflows/nightly-verify.yml | 5 ++++
c/driver/flightsql/sqlite_flightsql_test.cc | 16 +++++-----
c/driver/postgresql/postgresql_test.cc | 45 +++++++++++++++--------------
c/validation/adbc_validation_statement.cc | 24 ++++++++-------
c/vendor/nanoarrow/nanoarrow.hpp | 7 +++++
ci/docker/cpp-clang-latest.dockerfile | 12 ++++----
ci/docker/cpp-gcc-latest.dockerfile | 34 ++++++++++++++++++++++
docker-compose.yml | 15 ++++++++--
8 files changed, 112 insertions(+), 46 deletions(-)
diff --git a/.github/workflows/nightly-verify.yml
b/.github/workflows/nightly-verify.yml
index 3d27ef164..2d4652585 100644
--- a/.github/workflows/nightly-verify.yml
+++ b/.github/workflows/nightly-verify.yml
@@ -190,6 +190,11 @@ jobs:
pushd arrow-adbc
docker compose run --rm cpp-clang-latest
+ - name: cpp-gcc-latest
+ run: |
+ pushd arrow-adbc
+ docker compose run --rm cpp-gcc-latest
+
- name: python-debug
run: |
pushd arrow-adbc
diff --git a/c/driver/flightsql/sqlite_flightsql_test.cc
b/c/driver/flightsql/sqlite_flightsql_test.cc
index 4797d58e7..752ce8154 100644
--- a/c/driver/flightsql/sqlite_flightsql_test.cc
+++ b/c/driver/flightsql/sqlite_flightsql_test.cc
@@ -235,18 +235,20 @@ TEST_F(SqliteFlightSqlTest, TestGarbageInput) {
ASSERT_THAT(AdbcDatabaseRelease(&database, &error), IsOkStatus(&error));
}
+int Canary(const struct AdbcError*) { return 0; }
+
TEST_F(SqliteFlightSqlTest, AdbcDriverBackwardsCompatibility) {
- // XXX: sketchy cast
- auto* driver = static_cast<struct
AdbcDriver*>(malloc(ADBC_DRIVER_1_0_0_SIZE));
- std::memset(driver, 0, ADBC_DRIVER_1_0_0_SIZE);
+ struct AdbcDriver driver;
+ std::memset(&driver, 0, ADBC_DRIVER_1_1_0_SIZE);
+ driver.ErrorGetDetailCount = Canary;
- ASSERT_THAT(::FlightSQLDriverInit(ADBC_VERSION_1_0_0, driver, &error),
+ ASSERT_THAT(::FlightSQLDriverInit(ADBC_VERSION_1_0_0, &driver, &error),
IsOkStatus(&error));
- ASSERT_THAT(::FlightSQLDriverInit(424242, driver, &error),
- adbc_validation::IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
+ ASSERT_EQ(Canary, driver.ErrorGetDetailCount);
- free(driver);
+ ASSERT_THAT(::FlightSQLDriverInit(424242, &driver, &error),
+ adbc_validation::IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
}
class SqliteFlightSqlConnectionTest : public ::testing::Test,
diff --git a/c/driver/postgresql/postgresql_test.cc
b/c/driver/postgresql/postgresql_test.cc
index be32bd893..b76ba1cf7 100644
--- a/c/driver/postgresql/postgresql_test.cc
+++ b/c/driver/postgresql/postgresql_test.cc
@@ -223,18 +223,20 @@ class PostgresDatabaseTest : public ::testing::Test,
};
ADBCV_TEST_DATABASE(PostgresDatabaseTest)
+int Canary(const struct AdbcError*) { return 0; }
+
TEST_F(PostgresDatabaseTest, AdbcDriverBackwardsCompatibility) {
- // XXX: sketchy cast
- auto* driver = static_cast<struct
AdbcDriver*>(malloc(ADBC_DRIVER_1_0_0_SIZE));
- std::memset(driver, 0, ADBC_DRIVER_1_0_0_SIZE);
+ struct AdbcDriver driver;
+ std::memset(&driver, 0, ADBC_DRIVER_1_1_0_SIZE);
+ driver.ErrorGetDetailCount = Canary;
- ASSERT_THAT(::PostgresqlDriverInit(ADBC_VERSION_1_0_0, driver, &error),
+ ASSERT_THAT(::PostgresqlDriverInit(ADBC_VERSION_1_0_0, &driver, &error),
IsOkStatus(&error));
- ASSERT_THAT(::PostgresqlDriverInit(424242, driver, &error),
- IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
+ ASSERT_EQ(Canary, driver.ErrorGetDetailCount);
- free(driver);
+ ASSERT_THAT(::PostgresqlDriverInit(424242, &driver, &error),
+ IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
}
class PostgresConnectionTest : public ::testing::Test,
@@ -1552,24 +1554,25 @@ TEST_F(PostgresStatementTest, BatchSizeHint) {
// Test that an ADBC 1.0.0-sized error still works
TEST_F(PostgresStatementTest, AdbcErrorBackwardsCompatibility) {
- // XXX: sketchy cast
- auto* error = static_cast<struct AdbcError*>(malloc(ADBC_ERROR_1_0_0_SIZE));
- std::memset(error, 0, ADBC_ERROR_1_0_0_SIZE);
+ struct AdbcError error;
+ std::memset(&error, 0, ADBC_ERROR_1_1_0_SIZE);
+ struct AdbcDriver canary;
+ error.private_data = &canary;
+ error.private_driver = &canary;
- ASSERT_THAT(AdbcStatementNew(&connection, &statement, error),
IsOkStatus(error));
+ ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error),
IsOkStatus(&error));
ASSERT_THAT(
- AdbcStatementSetSqlQuery(&statement, "SELECT * FROM
thistabledoesnotexist", error),
- IsOkStatus(error));
+ AdbcStatementSetSqlQuery(&statement, "SELECT * FROM
thistabledoesnotexist", &error),
+ IsOkStatus(&error));
adbc_validation::StreamReader reader;
ASSERT_THAT(AdbcStatementExecuteQuery(&statement, &reader.stream.value,
- &reader.rows_affected, error),
- IsStatus(ADBC_STATUS_NOT_FOUND, error));
-
- ASSERT_EQ("42P01", std::string_view(error->sqlstate, 5));
- ASSERT_EQ(0, AdbcErrorGetDetailCount(error));
-
- error->release(error);
- free(error);
+ &reader.rows_affected, &error),
+ IsStatus(ADBC_STATUS_NOT_FOUND, &error));
+ ASSERT_EQ("42P01", std::string_view(error.sqlstate, 5));
+ ASSERT_EQ(0, AdbcErrorGetDetailCount(&error));
+ ASSERT_EQ(&canary, error.private_data);
+ ASSERT_EQ(&canary, error.private_driver);
+ error.release(&error);
}
TEST_F(PostgresStatementTest, Cancel) {
diff --git a/c/validation/adbc_validation_statement.cc
b/c/validation/adbc_validation_statement.cc
index cd388623b..8c6dd9484 100644
--- a/c/validation/adbc_validation_statement.cc
+++ b/c/validation/adbc_validation_statement.cc
@@ -2817,21 +2817,23 @@ struct ADBC_EXPORT AdbcError100 {
// Test that an ADBC 1.0.0-sized error still works
void StatementTest::TestErrorCompatibility() {
static_assert(sizeof(AdbcError100) == ADBC_ERROR_1_0_0_SIZE, "Wrong size");
- // XXX: sketchy cast
- auto* error = reinterpret_cast<struct
AdbcError*>(malloc(ADBC_ERROR_1_0_0_SIZE));
- std::memset(error, 0, ADBC_ERROR_1_0_0_SIZE);
+ struct AdbcError error;
+ std::memset(&error, 0, ADBC_ERROR_1_1_0_SIZE);
+ struct AdbcDriver canary;
+ error.private_data = &canary;
+ error.private_driver = &canary;
- ASSERT_THAT(AdbcStatementNew(&connection, &statement, error),
IsOkStatus(error));
+ ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error),
IsOkStatus(&error));
ASSERT_THAT(
- AdbcStatementSetSqlQuery(&statement, "SELECT * FROM
thistabledoesnotexist", error),
- IsOkStatus(error));
+ AdbcStatementSetSqlQuery(&statement, "SELECT * FROM
thistabledoesnotexist", &error),
+ IsOkStatus(&error));
adbc_validation::StreamReader reader;
ASSERT_THAT(AdbcStatementExecuteQuery(&statement, &reader.stream.value,
- &reader.rows_affected, error),
- ::testing::Not(IsOkStatus(error)));
- auto* old_error = reinterpret_cast<AdbcError100*>(error);
- old_error->release(old_error);
- free(error);
+ &reader.rows_affected, &error),
+ ::testing::Not(IsOkStatus(&error)));
+ ASSERT_EQ(&canary, error.private_data);
+ ASSERT_EQ(&canary, error.private_driver);
+ error.release(&error);
}
void StatementTest::TestResultInvalidation() {
diff --git a/c/vendor/nanoarrow/nanoarrow.hpp b/c/vendor/nanoarrow/nanoarrow.hpp
index 16c2e55b9..f2eade3d8 100644
--- a/c/vendor/nanoarrow/nanoarrow.hpp
+++ b/c/vendor/nanoarrow/nanoarrow.hpp
@@ -92,9 +92,16 @@ namespace literals {
/// @{
/// \brief User literal operator allowing ArrowStringView construction like
"str"_asv
+#if !defined(__clang__) && (defined(__GNUC__) && __GNUC__ < 6)
inline ArrowStringView operator"" _asv(const char* data, std::size_t
size_bytes) {
return {data, static_cast<int64_t>(size_bytes)};
}
+#else
+inline ArrowStringView operator""_asv(const char* data, std::size_t
size_bytes) {
+ return {data, static_cast<int64_t>(size_bytes)};
+}
+#endif
+// N.B. older GCC requires the space above, newer Clang forbids the space
// @}
diff --git a/ci/docker/cpp-clang-latest.dockerfile
b/ci/docker/cpp-clang-latest.dockerfile
index ff6e161e3..a1b76a552 100644
--- a/ci/docker/cpp-clang-latest.dockerfile
+++ b/ci/docker/cpp-clang-latest.dockerfile
@@ -15,9 +15,8 @@
# specific language governing permissions and limitations
# under the License.
-ARG VCPKG
-
FROM debian:12
+ARG GO
RUN export DEBIAN_FRONTEND=noninteractive && \
apt-get update -y && \
@@ -34,6 +33,9 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
RUN export DEBIAN_FRONTEND=noninteractive && \
apt-get install -y cmake git libpq-dev libsqlite3-dev pkg-config
-RUN curl -L -o go.tar.gz https://go.dev/dl/go1.22.5.linux-amd64.tar.gz && \
- tar -C /opt -xvf go.tar.gz && \
- echo 'export PATH=$PATH:/opt/go/bin' | tee -a ~/.bashrc
+RUN curl -L -o go.tar.gz https://go.dev/dl/go${GO}.linux-amd64.tar.gz && \
+ tar -C /opt -xvf go.tar.gz
+
+ENV PATH=/opt/go/bin:$PATH \
+ CC=/usr/bin/clang \
+ CXX=/usr/bin/clang++
diff --git a/ci/docker/cpp-gcc-latest.dockerfile
b/ci/docker/cpp-gcc-latest.dockerfile
new file mode 100644
index 000000000..38a7781ef
--- /dev/null
+++ b/ci/docker/cpp-gcc-latest.dockerfile
@@ -0,0 +1,34 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+FROM amd64/debian:experimental
+ARG GCC
+ARG GO
+
+ENV DEBIAN_FRONTEND noninteractive
+
+RUN apt-get update -y && \
+ apt-get install -y -q cmake curl git gnupg libpq-dev libsqlite3-dev
pkg-config && \
+ apt-get install -y -q -t experimental g++-${GCC} gcc-${GCC} && \
+ apt-get clean
+
+RUN curl -L -o go.tar.gz https://go.dev/dl/go${GO}.linux-amd64.tar.gz && \
+ tar -C /opt -xvf go.tar.gz
+
+ENV PATH=/opt/go/bin:$PATH \
+ CC=/usr/bin/gcc-${GCC} \
+ CXX=/usr/bin/g++-${GCC}
diff --git a/docker-compose.yml b/docker-compose.yml
index bf961cdb8..1872a2cfc 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -38,10 +38,21 @@ services:
context: .
dockerfile: ci/docker/cpp-clang-latest.dockerfile
args:
- VCPKG: ${VCPKG}
+ GO: ${GO}
+ volumes:
+ - .:/adbc:delegated
+ command: "bash -c 'git config --global --add safe.directory /adbc &&
/adbc/ci/scripts/cpp_build.sh /adbc /adbc/build/clang-latest && env BUILD_ALL=0
BUILD_DRIVER_MANAGER=1 BUILD_DRIVER_SQLITE=1 /adbc/ci/scripts/cpp_test.sh
/adbc/build/clang-latest'"
+
+ cpp-gcc-latest:
+ build:
+ context: .
+ dockerfile: ci/docker/cpp-gcc-latest.dockerfile
+ args:
+ GCC: 15
+ GO: ${GO}
volumes:
- .:/adbc:delegated
- command: "bash -c 'export PATH=$PATH:/opt/go/bin CC=$(which clang)
CXX=$(which clang++) && git config --global --add safe.directory /adbc &&
/adbc/ci/scripts/cpp_build.sh /adbc /adbc/build && env BUILD_ALL=0
BUILD_DRIVER_MANAGER=1 BUILD_DRIVER_SQLITE=1 /adbc/ci/scripts/cpp_test.sh
/adbc/build'"
+ command: "bash -c 'git config --global --add safe.directory /adbc &&
/adbc/ci/scripts/cpp_build.sh /adbc /adbc/build/gcc-latest && env BUILD_ALL=0
BUILD_DRIVER_MANAGER=1 BUILD_DRIVER_SQLITE=1 /adbc/ci/scripts/cpp_test.sh
/adbc/build/gcc-latest'"
############################ Documentation
###################################
