refactor(c/driver/framework): Remove fmt as required dependency of th…

…e driver framework (#2081)

This PR removes fmt as a required component of a driver based on the
driver framework. This is so that we can eliminate
`c/driver/common/driver_base.h` in favour of the much better
`c/driver/framework`.

The fmt library is great and easy to use and we should absolutely use it
in our own drivers; however, I don't know of any libraries that would
want to export an ADBC driver that don't already have a scheme for error
generation (e.g., Arrow's Status or GDAL's `cpl_error.h`). In the R
package we don't generate any fancy errors but we do need to make a
driver to check that the wires are plugged in. I think I've done this in
such a way that we keep all the benefits for our own drivers but drop
the requirement for all users.

This PR leans in to the `Status/Result`. I take back everything I ever
said about it...it's awesome and we should keep it (closes #1663). I
think it will be particularly nice for the consuming end as well if or
when we get there.

This PR also limits the use of nanoarrow to .cc files (i.e., nanoarrow
is not exposed in the headers). This mostly just seemed cleaner, if not
strictly necessary (/there are other problems that need solving like
namespacing if the internal nanoarrow were to be completely isolated). I
also removed references to any headers outside `driver/framework`
(mostly to the old utility library).

This is not quite far enough to do what I need to do in the R package,
where I basically just want an empty driver that does nothing. We could
fence the bits that rely on the other framework components with `#if
!defined(ADBC_FRAMEWORK_MINIMAL)` or separate `driver::BaseDatabase`
(contains no `XXXImpl()` methods) from `driver::Database` (what the
current BaseDatabase is).

c/driver/framework/CMakeLists.txt

@@ -25,20 +25,21 @@ target_include_directories(adbc_driver_framework
                                    "${REPOSITORY_ROOT}/c/vendor")
 target_link_libraries(adbc_driver_framework PUBLIC adbc_driver_common fmt::fmt)
 
-# if(ADBC_BUILD_TESTS)
-#   add_test_case(driver_framework_test
-#                 PREFIX
-#                 adbc
-#                 EXTRA_LABELS
-#                 driver-framework
-#                 SOURCES
-#                 utils_test.cc
-#                 driver_test.cc
-#                 EXTRA_LINK_LIBS
-#                 adbc_driver_framework
-#                 nanoarrow)
-#   target_compile_features(adbc-driver-framework-test PRIVATE cxx_std_17)
-#   target_include_directories(adbc-driver-framework-test
-#                              PRIVATE "${REPOSITORY_ROOT}" "${REPOSITORY_ROOT}/c/vendor")
-#   adbc_configure_target(adbc-driver-framework-test)
-# endif()
+if(ADBC_BUILD_TESTS)
+  add_test_case(driver_framework_test
+                PREFIX
+                adbc
+                EXTRA_LABELS
+                driver-framework
+                SOURCES
+                base_driver_test.cc
+                EXTRA_LINK_LIBS
+                adbc_driver_framework
+                nanoarrow)
+  target_compile_features(adbc-driver-framework-test PRIVATE cxx_std_17)
+  target_include_directories(adbc-driver-framework-test
+                             PRIVATE "${REPOSITORY_ROOT}/c/"
+                                     "${REPOSITORY_ROOT}/c/include"
+                                     "${REPOSITORY_ROOT}/c/vendor")
+  adbc_configure_target(adbc-driver-framework-test)
+endif()

c/driver/framework/base_connection.h

@@ -25,11 +25,7 @@
 #include <vector>
 
 #include <arrow-adbc/adbc.h>
-#include <nanoarrow/nanoarrow.h>
-#include <nanoarrow/nanoarrow.hpp>
 
-#include "driver/common/options.h"
-#include "driver/common/utils.h"
 #include "driver/framework/base_driver.h"
 #include "driver/framework/catalog.h"
 #include "driver/framework/objects.h"
@@ -71,8 +67,8 @@ class ConnectionBase : public ObjectBase {
   AdbcStatusCode Commit(AdbcError* error) {
     switch (autocommit_) {
       case AutocommitState::kAutocommit:
-        return status::InvalidState("{} No active transaction, cannot commit",
-                                    Derived::kErrorPrefix)
+        return status::InvalidState(Derived::kErrorPrefix,
+                                    " No active transaction, cannot commit")
             .ToAdbc(error);
       case AutocommitState::kTransaction:
         return impl().CommitImpl().ToAdbc(error);
@@ -84,36 +80,14 @@ class ConnectionBase : public ObjectBase {
   /// \internal
   AdbcStatusCode GetInfo(const uint32_t* info_codes, size_t info_codes_length,
                          ArrowArrayStream* out, AdbcError* error) {
-    std::vector<uint32_t> codes(info_codes, info_codes + info_codes_length);
-    RAISE_RESULT(error, auto infos, impl().InfoImpl(codes));
-
-    nanoarrow::UniqueSchema schema;
-    nanoarrow::UniqueArray array;
-    RAISE_STATUS(error, AdbcInitConnectionGetInfoSchema(schema.get(), array.get()));
-
-    for (const auto& info : infos) {
-      RAISE_STATUS(
-          error,
-          std::visit(
-              [&](auto&& value) -> Status {
-                using T = std::decay_t<decltype(value)>;
-                if constexpr (std::is_same_v<T, std::string>) {
-                  return AdbcConnectionGetInfoAppendString(array.get(), info.code, value);
-                } else if constexpr (std::is_same_v<T, int64_t>) {
-                  return AdbcConnectionGetInfoAppendInt(array.get(), info.code, value);
-                } else {
-                  static_assert(!sizeof(T), "info value type not implemented");
-                }
-                return status::Ok();
-              },
-              info.value));
-      CHECK_NA(INTERNAL, ArrowArrayFinishElement(array.get()), error);
+    if (!out) {
+      RAISE_STATUS(error, status::InvalidArgument("out must be non-null"));
     }
 
-    struct ArrowError na_error = {0};
-    CHECK_NA_DETAIL(INTERNAL, ArrowArrayFinishBuildingDefault(array.get(), &na_error),
-                    &na_error, error);
-    return BatchToArrayStream(array.get(), schema.get(), out, error);
+    std::vector<uint32_t> codes(info_codes, info_codes + info_codes_length);
+    RAISE_RESULT(error, auto infos, impl().InfoImpl(codes));
+    RAISE_STATUS(error, AdbcGetInfo(infos, out));
+    return ADBC_STATUS_OK;
   }
 
   /// \internal
@@ -152,20 +126,17 @@ class ConnectionBase : public ObjectBase {
         depth = GetObjectsDepth::kTables;
         break;
       default:
-        return status::InvalidArgument("{} GetObjects: invalid depth {}",
-                                       Derived::kErrorPrefix, c_depth)
+        return status::InvalidArgument(Derived::kErrorPrefix,
+                                       " GetObjects: invalid depth ", c_depth)
             .ToAdbc(error);
     }
 
     RAISE_RESULT(error, auto helper, impl().GetObjectsImpl());
-    nanoarrow::UniqueSchema schema;
-    nanoarrow::UniqueArray array;
-    auto status =
-        BuildGetObjects(helper.get(), depth, catalog_filter, schema_filter, table_filter,
-                        column_filter, table_type_filter, schema.get(), array.get());
+    auto status = BuildGetObjects(helper.get(), depth, catalog_filter, schema_filter,
+                                  table_filter, column_filter, table_type_filter, out);
     RAISE_STATUS(error, helper->Close());
     RAISE_STATUS(error, status);
-    return BatchToArrayStream(array.get(), schema.get(), out, error);
+    return ADBC_STATUS_OK;
   }
 
   /// \internal
@@ -210,8 +181,8 @@ class ConnectionBase : public ObjectBase {
                                 const char* table_name, ArrowSchema* schema,
                                 AdbcError* error) {
     if (!table_name) {
-      return status::InvalidArgument("{} GetTableSchema: must provide table_name",
-                                     Derived::kErrorPrefix)
+      return status::InvalidArgument(Derived::kErrorPrefix,
+                                     " GetTableSchema: must provide table_name")
           .ToAdbc(error);
     }
     std::memset(schema, 0, sizeof(*schema));
@@ -228,36 +199,13 @@ class ConnectionBase : public ObjectBase {
 
   /// \internal
   AdbcStatusCode GetTableTypes(ArrowArrayStream* out, AdbcError* error) {
-    RAISE_RESULT(error, std::vector<std::string> table_types, impl().GetTableTypesImpl());
-
-    nanoarrow::UniqueArray array;
-    nanoarrow::UniqueSchema schema;
-    ArrowSchemaInit(schema.get());
-
-    CHECK_NA(INTERNAL, ArrowSchemaSetType(schema.get(), NANOARROW_TYPE_STRUCT), error);
-    CHECK_NA(INTERNAL, ArrowSchemaAllocateChildren(schema.get(), /*num_columns=*/1),
-             error);
-    ArrowSchemaInit(schema.get()->children[0]);
-    CHECK_NA(INTERNAL,
-             ArrowSchemaSetType(schema.get()->children[0], NANOARROW_TYPE_STRING), error);
-    CHECK_NA(INTERNAL, ArrowSchemaSetName(schema.get()->children[0], "table_type"),
-             error);
-    schema.get()->children[0]->flags &= ~ARROW_FLAG_NULLABLE;
-
-    CHECK_NA(INTERNAL, ArrowArrayInitFromSchema(array.get(), schema.get(), NULL), error);
-    CHECK_NA(INTERNAL, ArrowArrayStartAppending(array.get()), error);
-
-    for (std::string const& table_type : table_types) {
-      CHECK_NA(
-          INTERNAL,
-          ArrowArrayAppendString(array->children[0], ArrowCharView(table_type.c_str())),
-          error);
-      CHECK_NA(INTERNAL, ArrowArrayFinishElement(array.get()), error);
+    if (!out) {
+      RAISE_STATUS(error, status::InvalidArgument("out must be non-null"));
     }
 
-    CHECK_NA(INTERNAL, ArrowArrayFinishBuildingDefault(array.get(), NULL), error);
-
-    return BatchToArrayStream(array.get(), schema.get(), out, error);
+    RAISE_RESULT(error, std::vector<std::string> table_types, impl().GetTableTypesImpl());
+    RAISE_STATUS(error, AdbcGetTableTypes(table_types, out));
+    return ADBC_STATUS_OK;
   }
 
   /// \internal
@@ -276,8 +224,8 @@ class ConnectionBase : public ObjectBase {
   AdbcStatusCode Rollback(AdbcError* error) {
     switch (autocommit_) {
       case AutocommitState::kAutocommit:
-        return status::InvalidState("{} No active transaction, cannot rollback",
-                                    Derived::kErrorPrefix)
+        return status::InvalidState(Derived::kErrorPrefix,
+                                    " No active transaction, cannot rollback")
             .ToAdbc(error);
       case AutocommitState::kTransaction:
         return impl().RollbackImpl().ToAdbc(error);
@@ -352,12 +300,12 @@ class ConnectionBase : public ObjectBase {
       }
       return status::Ok();
     }
-    return status::NotImplemented("{} Unknown connection option {}={}",
-                                  Derived::kErrorPrefix, key, value);
+    return status::NotImplemented(Derived::kErrorPrefix, " Unknown connection option ",
+                                  key, "=", value.Format());
   }
 
   Status ToggleAutocommitImpl(bool enable_autocommit) {
-    return status::NotImplemented("{} Cannot change autocommit", Derived::kErrorPrefix);
+    return status::NotImplemented(Derived::kErrorPrefix, " Cannot change autocommit");
   }
 
  protected:

c/driver/framework/base_database.h

@@ -68,8 +68,8 @@ class DatabaseBase : public ObjectBase {
 
   /// \brief Set an option.  May be called prior to InitImpl.
   virtual Status SetOptionImpl(std::string_view key, Option value) {
-    return status::NotImplemented("{} Unknown database option {}={}",
-                                  Derived::kErrorPrefix, key, value);
+    return status::NotImplemented(Derived::kErrorPrefix, " Unknown database option ", key,
+                                  "=", value.Format());
   }
 
  private:

c/driver/framework/base_driver.cc

@@ -29,7 +29,7 @@ Result<bool> Option::AsBool() const {
             return false;
           }
         }
-        return status::InvalidArgument("Invalid boolean value {}", *this);
+        return status::InvalidArgument("Invalid boolean value ", this->Format());
       },
       value_);
 }
@@ -46,15 +46,15 @@ Result<int64_t> Option::AsInt() const {
           auto end = value.data() + value.size();
           auto result = std::from_chars(begin, end, parsed);
           if (result.ec != std::errc()) {
-            return status::InvalidArgument("Invalid integer value '{}': not an integer",
-                                           value);
+            return status::InvalidArgument("Invalid integer value '", value,
+                                           "': not an integer", value);
           } else if (result.ptr != end) {
-            return status::InvalidArgument("Invalid integer value '{}': trailing data",
-                                           value);
+            return status::InvalidArgument("Invalid integer value '", value,
+                                           "': trailing data", value);
           }
           return parsed;
         }
-        return status::InvalidArgument("Invalid integer value {}", *this);
+        return status::InvalidArgument("Invalid integer value ", this->Format());
       },
       value_);
 }
@@ -66,7 +66,24 @@ Result<std::string_view> Option::AsString() const {
         if constexpr (std::is_same_v<T, std::string>) {
           return value;
         }
-        return status::InvalidArgument("Invalid string value {}", *this);
+        return status::InvalidArgument("Invalid string value {}", this->Format());
+      },
+      value_);
+}
+
+std::string Option::Format() const {
+  return std::visit(
+      [&](auto&& value) -> std::string {
+        using T = std::decay_t<decltype(value)>;
+        if constexpr (std::is_same_v<T, adbc::driver::Option::Unset>) {
+          return "(NULL)";
+        } else if constexpr (std::is_same_v<T, std::string>) {
+          return std::string("'") + value + "'";
+        } else if constexpr (std::is_same_v<T, std::vector<uint8_t>>) {
+          return std::string("(") + std::to_string(value.size()) + " bytes)";
+        } else {
+          return std::to_string(value);
+        }
       },
       value_);
 }
@@ -157,4 +174,5 @@ AdbcStatusCode Option::CGet(double* out, AdbcError* error) const {
       },
       value_);
 }
+
 }  // namespace adbc::driver

c/driver/framework/base_driver.h

@@ -29,10 +29,7 @@
 #include <vector>
 
 #include <arrow-adbc/adbc.h>
-#include <fmt/core.h>
-#include <fmt/format.h>
 
-#include "driver/common/utils.h"
 #include "driver/framework/status.h"
 
 /// \file base.h ADBC Driver Framework
@@ -91,6 +88,9 @@ class Option {
   /// \brief Get the value if it is a string.
   Result<std::string_view> AsString() const;
 
+  /// \brief Provide a human-readable summary of the value
+  std::string Format() const;
+
  private:
   Value value_;
 
@@ -623,26 +623,3 @@ class Driver {
 };
 
 }  // namespace adbc::driver
-
-/// \brief Formatter for Option values.
-template <>
-struct fmt::formatter<adbc::driver::Option> : fmt::nested_formatter<std::string_view> {
-  auto format(const adbc::driver::Option& option, fmt::format_context& ctx) const {
-    return write_padded(ctx, [=](auto out) {
-      return std::visit(
-          [&](auto&& value) {
-            using T = std::decay_t<decltype(value)>;
-            if constexpr (std::is_same_v<T, adbc::driver::Option::Unset>) {
-              return fmt::format_to(out, "(NULL)");
-            } else if constexpr (std::is_same_v<T, std::string>) {
-              return fmt::format_to(out, "'{}'", value);
-            } else if constexpr (std::is_same_v<T, std::vector<uint8_t>>) {
-              return fmt::format_to(out, "({} bytes)", value.size());
-            } else {
-              return fmt::format_to(out, "{}", value);
-            }
-          },
-          option.value());
-    });
-  }
-};