Skip to content

Commit 63bb903

Browse files
lidavidmkou
andauthored
test(c): don't use sketchy cast to test backwards compatibility (apache#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 apache#2424. --------- Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
1 parent 196522b commit 63bb903

File tree

8 files changed

+112
-46
lines changed

8 files changed

+112
-46
lines changed

.github/workflows/nightly-verify.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,11 @@ jobs:
190190
pushd arrow-adbc
191191
docker compose run --rm cpp-clang-latest
192192
193+
- name: cpp-gcc-latest
194+
run: |
195+
pushd arrow-adbc
196+
docker compose run --rm cpp-gcc-latest
197+
193198
- name: python-debug
194199
run: |
195200
pushd arrow-adbc

c/driver/flightsql/sqlite_flightsql_test.cc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -235,18 +235,20 @@ TEST_F(SqliteFlightSqlTest, TestGarbageInput) {
235235
ASSERT_THAT(AdbcDatabaseRelease(&database, &error), IsOkStatus(&error));
236236
}
237237

238+
int Canary(const struct AdbcError*) { return 0; }
239+
238240
TEST_F(SqliteFlightSqlTest, AdbcDriverBackwardsCompatibility) {
239-
// XXX: sketchy cast
240-
auto* driver = static_cast<struct AdbcDriver*>(malloc(ADBC_DRIVER_1_0_0_SIZE));
241-
std::memset(driver, 0, ADBC_DRIVER_1_0_0_SIZE);
241+
struct AdbcDriver driver;
242+
std::memset(&driver, 0, ADBC_DRIVER_1_1_0_SIZE);
243+
driver.ErrorGetDetailCount = Canary;
242244

243-
ASSERT_THAT(::FlightSQLDriverInit(ADBC_VERSION_1_0_0, driver, &error),
245+
ASSERT_THAT(::FlightSQLDriverInit(ADBC_VERSION_1_0_0, &driver, &error),
244246
IsOkStatus(&error));
245247

246-
ASSERT_THAT(::FlightSQLDriverInit(424242, driver, &error),
247-
adbc_validation::IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
248+
ASSERT_EQ(Canary, driver.ErrorGetDetailCount);
248249

249-
free(driver);
250+
ASSERT_THAT(::FlightSQLDriverInit(424242, &driver, &error),
251+
adbc_validation::IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
250252
}
251253

252254
class SqliteFlightSqlConnectionTest : public ::testing::Test,

c/driver/postgresql/postgresql_test.cc

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -223,18 +223,20 @@ class PostgresDatabaseTest : public ::testing::Test,
223223
};
224224
ADBCV_TEST_DATABASE(PostgresDatabaseTest)
225225

226+
int Canary(const struct AdbcError*) { return 0; }
227+
226228
TEST_F(PostgresDatabaseTest, AdbcDriverBackwardsCompatibility) {
227-
// XXX: sketchy cast
228-
auto* driver = static_cast<struct AdbcDriver*>(malloc(ADBC_DRIVER_1_0_0_SIZE));
229-
std::memset(driver, 0, ADBC_DRIVER_1_0_0_SIZE);
229+
struct AdbcDriver driver;
230+
std::memset(&driver, 0, ADBC_DRIVER_1_1_0_SIZE);
231+
driver.ErrorGetDetailCount = Canary;
230232

231-
ASSERT_THAT(::PostgresqlDriverInit(ADBC_VERSION_1_0_0, driver, &error),
233+
ASSERT_THAT(::PostgresqlDriverInit(ADBC_VERSION_1_0_0, &driver, &error),
232234
IsOkStatus(&error));
233235

234-
ASSERT_THAT(::PostgresqlDriverInit(424242, driver, &error),
235-
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
236+
ASSERT_EQ(Canary, driver.ErrorGetDetailCount);
236237

237-
free(driver);
238+
ASSERT_THAT(::PostgresqlDriverInit(424242, &driver, &error),
239+
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
238240
}
239241

240242
class PostgresConnectionTest : public ::testing::Test,
@@ -1552,24 +1554,25 @@ TEST_F(PostgresStatementTest, BatchSizeHint) {
15521554

15531555
// Test that an ADBC 1.0.0-sized error still works
15541556
TEST_F(PostgresStatementTest, AdbcErrorBackwardsCompatibility) {
1555-
// XXX: sketchy cast
1556-
auto* error = static_cast<struct AdbcError*>(malloc(ADBC_ERROR_1_0_0_SIZE));
1557-
std::memset(error, 0, ADBC_ERROR_1_0_0_SIZE);
1557+
struct AdbcError error;
1558+
std::memset(&error, 0, ADBC_ERROR_1_1_0_SIZE);
1559+
struct AdbcDriver canary;
1560+
error.private_data = &canary;
1561+
error.private_driver = &canary;
15581562

1559-
ASSERT_THAT(AdbcStatementNew(&connection, &statement, error), IsOkStatus(error));
1563+
ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error));
15601564
ASSERT_THAT(
1561-
AdbcStatementSetSqlQuery(&statement, "SELECT * FROM thistabledoesnotexist", error),
1562-
IsOkStatus(error));
1565+
AdbcStatementSetSqlQuery(&statement, "SELECT * FROM thistabledoesnotexist", &error),
1566+
IsOkStatus(&error));
15631567
adbc_validation::StreamReader reader;
15641568
ASSERT_THAT(AdbcStatementExecuteQuery(&statement, &reader.stream.value,
1565-
&reader.rows_affected, error),
1566-
IsStatus(ADBC_STATUS_NOT_FOUND, error));
1567-
1568-
ASSERT_EQ("42P01", std::string_view(error->sqlstate, 5));
1569-
ASSERT_EQ(0, AdbcErrorGetDetailCount(error));
1570-
1571-
error->release(error);
1572-
free(error);
1569+
&reader.rows_affected, &error),
1570+
IsStatus(ADBC_STATUS_NOT_FOUND, &error));
1571+
ASSERT_EQ("42P01", std::string_view(error.sqlstate, 5));
1572+
ASSERT_EQ(0, AdbcErrorGetDetailCount(&error));
1573+
ASSERT_EQ(&canary, error.private_data);
1574+
ASSERT_EQ(&canary, error.private_driver);
1575+
error.release(&error);
15731576
}
15741577

15751578
TEST_F(PostgresStatementTest, Cancel) {

c/validation/adbc_validation_statement.cc

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2817,21 +2817,23 @@ struct ADBC_EXPORT AdbcError100 {
28172817
// Test that an ADBC 1.0.0-sized error still works
28182818
void StatementTest::TestErrorCompatibility() {
28192819
static_assert(sizeof(AdbcError100) == ADBC_ERROR_1_0_0_SIZE, "Wrong size");
2820-
// XXX: sketchy cast
2821-
auto* error = reinterpret_cast<struct AdbcError*>(malloc(ADBC_ERROR_1_0_0_SIZE));
2822-
std::memset(error, 0, ADBC_ERROR_1_0_0_SIZE);
2820+
struct AdbcError error;
2821+
std::memset(&error, 0, ADBC_ERROR_1_1_0_SIZE);
2822+
struct AdbcDriver canary;
2823+
error.private_data = &canary;
2824+
error.private_driver = &canary;
28232825

2824-
ASSERT_THAT(AdbcStatementNew(&connection, &statement, error), IsOkStatus(error));
2826+
ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error));
28252827
ASSERT_THAT(
2826-
AdbcStatementSetSqlQuery(&statement, "SELECT * FROM thistabledoesnotexist", error),
2827-
IsOkStatus(error));
2828+
AdbcStatementSetSqlQuery(&statement, "SELECT * FROM thistabledoesnotexist", &error),
2829+
IsOkStatus(&error));
28282830
adbc_validation::StreamReader reader;
28292831
ASSERT_THAT(AdbcStatementExecuteQuery(&statement, &reader.stream.value,
2830-
&reader.rows_affected, error),
2831-
::testing::Not(IsOkStatus(error)));
2832-
auto* old_error = reinterpret_cast<AdbcError100*>(error);
2833-
old_error->release(old_error);
2834-
free(error);
2832+
&reader.rows_affected, &error),
2833+
::testing::Not(IsOkStatus(&error)));
2834+
ASSERT_EQ(&canary, error.private_data);
2835+
ASSERT_EQ(&canary, error.private_driver);
2836+
error.release(&error);
28352837
}
28362838

28372839
void StatementTest::TestResultInvalidation() {

c/vendor/nanoarrow/nanoarrow.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,16 @@ namespace literals {
9292
/// @{
9393

9494
/// \brief User literal operator allowing ArrowStringView construction like "str"_asv
95+
#if !defined(__clang__) && (defined(__GNUC__) && __GNUC__ < 6)
9596
inline ArrowStringView operator"" _asv(const char* data, std::size_t size_bytes) {
9697
return {data, static_cast<int64_t>(size_bytes)};
9798
}
99+
#else
100+
inline ArrowStringView operator""_asv(const char* data, std::size_t size_bytes) {
101+
return {data, static_cast<int64_t>(size_bytes)};
102+
}
103+
#endif
104+
// N.B. older GCC requires the space above, newer Clang forbids the space
98105

99106
// @}
100107

ci/docker/cpp-clang-latest.dockerfile

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18-
ARG VCPKG
19-
2018
FROM debian:12
19+
ARG GO
2120

2221
RUN export DEBIAN_FRONTEND=noninteractive && \
2322
apt-get update -y && \
@@ -34,6 +33,9 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
3433
RUN export DEBIAN_FRONTEND=noninteractive && \
3534
apt-get install -y cmake git libpq-dev libsqlite3-dev pkg-config
3635

37-
RUN curl -L -o go.tar.gz https://go.dev/dl/go1.22.5.linux-amd64.tar.gz && \
38-
tar -C /opt -xvf go.tar.gz && \
39-
echo 'export PATH=$PATH:/opt/go/bin' | tee -a ~/.bashrc
36+
RUN curl -L -o go.tar.gz https://go.dev/dl/go${GO}.linux-amd64.tar.gz && \
37+
tar -C /opt -xvf go.tar.gz
38+
39+
ENV PATH=/opt/go/bin:$PATH \
40+
CC=/usr/bin/clang \
41+
CXX=/usr/bin/clang++
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
FROM amd64/debian:experimental
19+
ARG GCC
20+
ARG GO
21+
22+
ENV DEBIAN_FRONTEND noninteractive
23+
24+
RUN apt-get update -y && \
25+
apt-get install -y -q cmake curl git gnupg libpq-dev libsqlite3-dev pkg-config && \
26+
apt-get install -y -q -t experimental g++-${GCC} gcc-${GCC} && \
27+
apt-get clean
28+
29+
RUN curl -L -o go.tar.gz https://go.dev/dl/go${GO}.linux-amd64.tar.gz && \
30+
tar -C /opt -xvf go.tar.gz
31+
32+
ENV PATH=/opt/go/bin:$PATH \
33+
CC=/usr/bin/gcc-${GCC} \
34+
CXX=/usr/bin/g++-${GCC}

docker-compose.yml

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,21 @@ services:
3838
context: .
3939
dockerfile: ci/docker/cpp-clang-latest.dockerfile
4040
args:
41-
VCPKG: ${VCPKG}
41+
GO: ${GO}
42+
volumes:
43+
- .:/adbc:delegated
44+
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'"
45+
46+
cpp-gcc-latest:
47+
build:
48+
context: .
49+
dockerfile: ci/docker/cpp-gcc-latest.dockerfile
50+
args:
51+
GCC: 15
52+
GO: ${GO}
4253
volumes:
4354
- .:/adbc:delegated
44-
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'"
55+
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'"
4556

4657
############################ Documentation ###################################
4758

0 commit comments

Comments
 (0)