Fixed adbc driver and force tests to run

Author: evertlammerts

CMakeLists.txt

@@ -79,6 +79,37 @@ pybind11_add_module(
 # add _duckdb_dependencies
 target_link_libraries(_duckdb PRIVATE _duckdb_dependencies)
 
+# ────────────────────────────────────────────
+# Controlling symbol export
+#
+# We want to export exactly two symbols: - PyInit__duckdb: this allows CPython
+# to load the module - duckdb_adbc_init: the DuckDB ADBC driver
+#
+# The export of symbols on OSX and Linux is controlled by: - Visibility
+# annotations in the code (for this lib we use the PYBIND11_EXPORT macro) -
+# Telling the linker which symbols we want exported, which we do below
+#
+# For Windows, we rely on just the visbility annotations.
+# ────────────────────────────────────────────
+set_target_properties(
+  _duckdb
+  PROPERTIES CXX_VISIBILITY_PRESET hidden
+             C_VISIBILITY_PRESET hidden
+             VISIBILITY_INLINES_HIDDEN ON)
+
+if(APPLE)
+  target_link_options(
+    _duckdb PRIVATE "LINKER:-exported_symbol,_duckdb_adbc_init"
+    "LINKER:-exported_symbol,_PyInit__duckdb")
+elseif(UNIX AND NOT APPLE)
+  target_link_options(
+    _duckdb PRIVATE "LINKER:--export-dynamic-symbol=duckdb_adbc_init"
+    "LINKER:--export-dynamic-symbol=PyInit__duckdb")
+endif()
+
+# if(MSVC) target_link_options(_duckdb PRIVATE "/INCLUDE:duckdb_adbc_init")
+# else() target_link_options(_duckdb PRIVATE "-Wl,-u,duckdb_adbc_init") endif()
+
 # ────────────────────────────────────────────
 # Put the object file in the correct place
 # ────────────────────────────────────────────

adbc_driver_duckdb/__init__.py

@@ -19,12 +19,11 @@
 
 import enum
 import functools
+import importlib
 import typing
 
 import adbc_driver_manager
 
-__all__ = ["StatementOptions", "connect"]
-
 
 class StatementOptions(enum.Enum):
     """Statement options specific to the DuckDB driver."""
@@ -36,12 +35,16 @@ class StatementOptions(enum.Enum):
 def connect(path: typing.Optional[str] = None) -> adbc_driver_manager.AdbcDatabase:
     """Create a low level ADBC connection to DuckDB."""
     if path is None:
-        return adbc_driver_manager.AdbcDatabase(driver=_driver_path(), entrypoint="duckdb_adbc_init")
-    return adbc_driver_manager.AdbcDatabase(driver=_driver_path(), entrypoint="duckdb_adbc_init", path=path)
+        return adbc_driver_manager.AdbcDatabase(driver=driver_path(), entrypoint="duckdb_adbc_init")
+    return adbc_driver_manager.AdbcDatabase(driver=driver_path(), entrypoint="duckdb_adbc_init", path=path)
 
 
 @functools.cache
-def _driver_path() -> str:
-    import duckdb
-
-    return duckdb.duckdb.__file__
+def driver_path() -> str:
+    """Get the path to the DuckDB ADBC driver."""
+    duckdb_module_spec = importlib.util.find_spec("_duckdb")
+    if duckdb_module_spec is None:
+        msg = "Could not find duckdb shared library. Did you pip install duckdb?"
+        raise ImportError(msg)
+    print(f"Found duckdb shared library at {duckdb_module_spec.origin}")
+    return duckdb_module_spec.origin

pyproject.toml

@@ -48,7 +48,7 @@ all = [ # users can install duckdb with 'duckdb[all]', which will install this l
     "numpy",   # used in duckdb.experimental.spark and in duckdb.fetchnumpy()
     "pandas",  # used for pandas dataframes all over the place
     "pyarrow", # used for pyarrow support
-    "adbc_driver_manager", # for the adbc driver (TODO: this should live under the duckdb package)
+    "adbc-driver-manager", # for the adbc driver
 ]
 
 ######################################################################################################
@@ -225,6 +225,7 @@ stubdeps = [ # dependencies used for typehints in the stubs
     "pyarrow",
 ]
 test = [ # dependencies used for running tests
+    "adbc-driver-manager",
     "pytest",
     "pytest-reraise",
     "pytest-timeout",

src/duckdb_py/duckdb_python.cpp

@@ -1007,7 +1007,15 @@ static void RegisterExpectedResultType(py::handle &m) {
 	expected_return_type.export_values();
 }
 
+/**
+ * Fwd declaration of duckdb_adbc_init (the entrypoint of our ADBC driver) that must be exported.
+ */
+extern "C" PYBIND11_EXPORT extern void duckdb_adbc_init(void);
+
 PYBIND11_MODULE(DUCKDB_PYTHON_LIB_NAME, m) { // NOLINT
+	// Take address to force duckdb_adbc_init symbol inclusion
+	(void)&duckdb_adbc_init;
+
 	py::enum_<duckdb::ExplainType>(m, "ExplainType")
 	    .value("STANDARD", duckdb::ExplainType::EXPLAIN_STANDARD)
 	    .value("ANALYZE", duckdb::ExplainType::EXPLAIN_ANALYZE)

tests/fast/adbc/test_adbc.py

@@ -1,24 +1,21 @@
 import datetime
 from pathlib import Path
 
+import adbc_driver_manager.dbapi
 import numpy as np
+import pyarrow
 import pytest
 
-import duckdb
-
-adbc_driver_manager = pytest.importorskip("adbc_driver_manager.dbapi")
-adbc_driver_manager_lib = pytest.importorskip("adbc_driver_manager._lib")
-
-pyarrow = pytest.importorskip("pyarrow")
+import adbc_driver_duckdb.dbapi
 
 # When testing local, if you build via BUILD_PYTHON=1 make, you need to manually set up the
 # dylib duckdb path.
-driver_path = duckdb.duckdb.__file__
+driver_path = adbc_driver_duckdb.driver_path()
 
 
 @pytest.fixture
 def duck_conn():
-    with adbc_driver_manager.connect(driver=driver_path, entrypoint="duckdb_adbc_init") as conn:
+    with adbc_driver_manager.dbapi.connect(driver=driver_path, entrypoint="duckdb_adbc_init") as conn:
         yield conn
 
 
@@ -98,7 +95,7 @@ def test_commit(tmp_path):
     table = example_table()
     db_kwargs = {"path": f"{db}"}
     # Start connection with auto-commit off
-    with adbc_driver_manager.connect(
+    with adbc_driver_manager.dbapi.connect(
         driver=driver_path,
         entrypoint="duckdb_adbc_init",
         db_kwargs=db_kwargs,
@@ -108,7 +105,7 @@ def test_commit(tmp_path):
             cur.adbc_ingest("ingest", table, "create")
 
     # Check Data is not there
-    with adbc_driver_manager.connect(
+    with adbc_driver_manager.dbapi.connect(
         driver=driver_path,
         entrypoint="duckdb_adbc_init",
         db_kwargs=db_kwargs,
@@ -118,7 +115,7 @@ def test_commit(tmp_path):
         with conn.cursor() as cur:
             # This errors because the table does not exist
             with pytest.raises(
-                adbc_driver_manager_lib.InternalError,
+                adbc_driver_manager._lib.InternalError,
                 match=r"Table with name ingest does not exist!",
             ):
                 cur.execute("SELECT count(*) from ingest")
@@ -127,7 +124,7 @@ def test_commit(tmp_path):
 
     # This now works because we enabled autocommit
     with (
-        adbc_driver_manager.connect(
+        adbc_driver_manager.dbapi.connect(
             driver=driver_path,
             entrypoint="duckdb_adbc_init",
             db_kwargs=db_kwargs,
@@ -221,7 +218,7 @@ def test_insertion(duck_conn):
     # Test Append
     with duck_conn.cursor() as cursor:
         with pytest.raises(
-            adbc_driver_manager_lib.InternalError,
+            adbc_driver_manager.InternalError,
             match=r'Table with name "ingest_table" already exists!',
         ):
             cursor.adbc_ingest("ingest_table", table, "create")
@@ -299,7 +296,7 @@ def test_large_chunk(tmp_path):
         db.unlink()
     db_kwargs = {"path": f"{db}"}
     with (
-        adbc_driver_manager.connect(
+        adbc_driver_manager.dbapi.connect(
             driver=driver_path,
             entrypoint="duckdb_adbc_init",
             db_kwargs=db_kwargs,
@@ -325,7 +322,7 @@ def test_dictionary_data(tmp_path):
         db.unlink()
     db_kwargs = {"path": f"{db}"}
     with (
-        adbc_driver_manager.connect(
+        adbc_driver_manager.dbapi.connect(
             driver=driver_path,
             entrypoint="duckdb_adbc_init",
             db_kwargs=db_kwargs,
@@ -353,7 +350,7 @@ def test_ree_data(tmp_path):
         db.unlink()
     db_kwargs = {"path": f"{db}"}
     with (
-        adbc_driver_manager.connect(
+        adbc_driver_manager.dbapi.connect(
             driver=driver_path,
             entrypoint="duckdb_adbc_init",
             db_kwargs=db_kwargs,

tests/fast/adbc/test_connection_get_info.py

@@ -1,31 +1,19 @@
-import pytest
+import pyarrow as pa
 
+import adbc_driver_duckdb.dbapi
 import duckdb
 
-pa = pytest.importorskip("pyarrow")
-adbc_driver_manager = pytest.importorskip("adbc_driver_manager")
-
-
-try:
-    adbc_driver_duckdb = pytest.importorskip("adbc_driver_duckdb.dbapi")
-    con = adbc_driver_duckdb.connect()
-except adbc_driver_manager.InternalError as e:
-    pytest.skip(
-        f"'duckdb_adbc_init' was not exported in this install, try running 'python3 setup.py install': {e}",
-        allow_module_level=True,
-    )
-
 
 class TestADBCConnectionGetInfo:
     def test_connection_basic(self):
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         with con.cursor() as cursor:
             cursor.execute("select 42")
             res = cursor.fetchall()
             assert res == [(42,)]
 
     def test_connection_get_info_all(self):
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         adbc_con = con.adbc_connection
         res = adbc_con.get_info()
         reader = pa.RecordBatchReader._import_from_c(res.address)
@@ -35,7 +23,7 @@ def test_connection_get_info_all(self):
         expected_result = pa.array(
             [
                 "duckdb",
-                "v" + duckdb.__version__,  # don't hardcode this, as it will change every version
+                "v" + duckdb.duckdb_version,  # don't hardcode this, as it will change every version
                 "ADBC DuckDB Driver",
                 "(unknown)",
                 "(unknown)",
@@ -49,7 +37,7 @@ def test_connection_get_info_all(self):
         assert string_values == expected_result
 
     def test_empty_result(self):
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         adbc_con = con.adbc_connection
         res = adbc_con.get_info([1337])
         reader = pa.RecordBatchReader._import_from_c(res.address)
@@ -60,7 +48,7 @@ def test_empty_result(self):
         assert values.num_chunks == 0
 
     def test_unrecognized_codes(self):
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         adbc_con = con.adbc_connection
         res = adbc_con.get_info([0, 1000, 4, 2000])
         reader = pa.RecordBatchReader._import_from_c(res.address)

tests/fast/adbc/test_statement_bind.py

@@ -1,10 +1,8 @@
+import adbc_driver_manager
+import pyarrow as pa
 import pytest
 
-pa = pytest.importorskip("pyarrow")
-adbc_driver_manager = pytest.importorskip("adbc_driver_manager")
-
-adbc_driver_duckdb = pytest.importorskip("adbc_driver_duckdb.dbapi")
-con = adbc_driver_duckdb.connect()
+import adbc_driver_duckdb.dbapi
 
 
 def _import(handle):
@@ -33,7 +31,7 @@ def test_bind_multiple_rows(self):
             names=["ints"],
         )
 
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         with con.cursor() as cursor:
             statement = cursor.adbc_statement
             statement.set_sql_query("select ? * 2 as i")
@@ -55,7 +53,7 @@ def test_bind_single_row(self):
             names=["ints"],
         )
 
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         with con.cursor() as cursor:
             statement = cursor.adbc_statement
             statement.set_sql_query("select ? * 2 as i")
@@ -90,7 +88,7 @@ def test_multiple_parameters(self):
             names=["ints", "strings", "bools"],
         )
 
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         with con.cursor() as cursor:
             statement = cursor.adbc_statement
             statement.set_sql_query("select ? as a, ? as b, ? as c")
@@ -120,7 +118,7 @@ def test_bind_composite_type(self):
         # Create the RecordBatch
         record_batch = pa.RecordBatch.from_arrays([struct_array], schema=schema)
 
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         with con.cursor() as cursor:
             statement = cursor.adbc_statement
             statement.set_sql_query("select ? as a")
@@ -143,7 +141,7 @@ def test_too_many_parameters(self):
             names=["ints", "strings"],
         )
 
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         with con.cursor() as cursor:
             statement = cursor.adbc_statement
             statement.set_sql_query("select ? as a")
@@ -171,7 +169,7 @@ def test_not_enough_parameters(self):
             names=["strings"],
         )
 
-        con = adbc_driver_duckdb.connect()
+        con = adbc_driver_duckdb.dbapi.connect()
         with con.cursor() as cursor:
             statement = cursor.adbc_statement
             statement.set_sql_query("select ? as a, ? as b")