Skip to content

Commit

Permalink
test(c/driver/sqlite): ensure that test suite passes with system-prov…
Browse files Browse the repository at this point in the history
…ided SQLite on MacOS (#1499)

I opened up the C drivers and ran the tests and got a failing test: when
the driver compiles against system SQLite on MacOS, extension loading is
not allowed (the symbols aren't even provided nor defined in the
header). I added a define to make sure the R package could build on
MacOS, but the test needs it too.

I also added come CMake to check this...I also tried
`check_sybmol_exists` but this seems to require setting
`CMAKE_REQUIRED_LIBRARIES`/includes which seems like it might have some
global impact that we don't want. Checking for the text in the header
does the trick for me (and is maybe less likely to result in
accidentally building the library without extension support).

The other failing test I saw before seems to have been cleared up by the
SQLite driver refactor!

The `.cache` in the `.gitignore` is because clangd seems to put a lot of
files there.
  • Loading branch information
paleolimbot authored Apr 5, 2024
1 parent 8b9dae0 commit dff48ec
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ MANIFEST
compile_commands.json
build.ninja
.clangd
.cache

# Generated Visual Studio files
*.vcxproj
Expand Down
16 changes: 15 additions & 1 deletion c/driver/sqlite/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ else()
set(SQLite3_INCLUDE_DIRS)
endif()

# Check for sqlite3_load_extension() in sqlite3.h
if(EXISTS "${SQLite3_INCLUDE_DIRS}/sqlite3.h")
file(READ "${SQLite3_INCLUDE_DIRS}/sqlite3.h" ADBC_SQLITE_H_CONTENT)
string(FIND "${ADBC_SQLITE_H_CONTENT}" "sqlite3_load_extension"
ADBC_SQLITE_WITH_LOAD_EXTENSION)
endif()

if(NOT ADBC_SQLITE_WITH_LOAD_EXTENSION)
set(ADBC_SQLITE_COMPILE_DEFINES "-DADBC_SQLITE_WITH_NO_LOAD_EXTENSION")
endif()

add_arrow_lib(adbc_driver_sqlite
SOURCES
sqlite.cc
Expand All @@ -50,7 +61,8 @@ add_arrow_lib(adbc_driver_sqlite
${LIBPQ_STATIC_LIBRARIES})

foreach(LIB_TARGET ${ADBC_LIBRARIES})
target_compile_definitions(${LIB_TARGET} PRIVATE ADBC_EXPORTING)
target_compile_definitions(${LIB_TARGET} PRIVATE ADBC_EXPORTING
${ADBC_SQLITE_COMPILE_DEFINES})
target_include_directories(${LIB_TARGET} SYSTEM
PRIVATE ${REPOSITORY_ROOT}
${REPOSITORY_ROOT}/c/
Expand Down Expand Up @@ -82,6 +94,8 @@ if(ADBC_BUILD_TESTS)
adbc_validation
nanoarrow
${TEST_LINK_LIBS})
target_compile_definitions(adbc-driver-sqlite-test
PRIVATE ${ADBC_SQLITE_COMPILE_DEFINES})
target_compile_features(adbc-driver-sqlite-test PRIVATE cxx_std_17)
target_include_directories(adbc-driver-sqlite-test SYSTEM
PRIVATE ${REPOSITORY_ROOT}
Expand Down
4 changes: 4 additions & 0 deletions c/driver/sqlite/sqlite_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ class SqliteConnectionTest : public ::testing::Test,
ADBCV_TEST_CONNECTION(SqliteConnectionTest)

TEST_F(SqliteConnectionTest, ExtensionLoading) {
#if defined(ADBC_SQLITE_WITH_NO_LOAD_EXTENSION)
GTEST_SKIP() << "Linking to SQLite without extension loading";
#endif

ASSERT_THAT(AdbcConnectionNew(&connection, &error),
adbc_validation::IsOkStatus(&error));

Expand Down

0 comments on commit dff48ec

Please sign in to comment.