Merge pull request #118 from samansmink/main-into-feature

Main into feature

.clang-format

@@ -0,0 +1 @@
+duckdb/.clang-format

.github/workflows/MainDistributionPipeline.yml

@@ -21,7 +21,7 @@ jobs:
       ci_tools_version: main
       extension_name: delta
       enable_rust: true
-      exclude_archs: 'wasm_mvp;wasm_eh;wasm_threads;windows_amd64_mingw'
+      exclude_archs: 'wasm_mvp;wasm_eh;wasm_threads;windows_amd64_rtools;windows_amd64_mingw'
       extra_toolchains: 'python3'
       vcpkg_commit: c82f74667287d3dc386bce81e44964370c91a289
 
@@ -32,6 +32,7 @@ jobs:
     secrets: inherit
     with:
       extension_name: delta
+      ci_tools_version: main
       duckdb_version: 0ccf3c25cc
-      exclude_archs: 'wasm_mvp;wasm_eh;wasm_threads;windows_amd64_mingw'
+      exclude_archs: 'wasm_mvp;wasm_eh;wasm_threads;windows_amd64_rtools;windows_amd64_mingw'
       deploy_latest: ${{ startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main' }}

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 2.8.12)
 include(ExternalProject)
 
-### Core config
+# Core config
 set(TARGET_NAME delta)
 
 set(EXTENSION_NAME ${TARGET_NAME}_extension)
@@ -12,30 +12,43 @@ project(${TARGET_NAME})
 include_directories(src/include)
 
 set(EXTENSION_SOURCES
-        src/delta_extension.cpp
-        src/delta_functions.cpp
-        src/delta_utils.cpp
-        src/functions/delta_scan.cpp
-        src/storage/delta_catalog.cpp
-        src/storage/delta_schema_entry.cpp
-        src/storage/delta_table_entry.cpp
-        src/storage/delta_transaction.cpp
-        src/storage/delta_transaction_manager.cpp
-)
-
-### Custom config
-# TODO: figure out if we really need this?
+    src/delta_extension.cpp
+    src/delta_functions.cpp
+    src/delta_utils.cpp
+    src/functions/delta_scan.cpp
+    src/storage/delta_catalog.cpp
+    src/storage/delta_schema_entry.cpp
+    src/storage/delta_table_entry.cpp
+    src/storage/delta_transaction.cpp
+    src/storage/delta_transaction_manager.cpp)
+
+# Custom config TODO: figure out if we really need this?
 if(APPLE)
-    set(PLATFORM_LIBS m c System resolv "-framework Corefoundation -framework SystemConfiguration -framework Security")
+  set(PLATFORM_LIBS
+      m
+      c
+      System
+      resolv
+      "-framework Corefoundation -framework SystemConfiguration -framework Security"
+  )
 elseif(UNIX)
-    set(PLATFORM_LIBS m c resolv)
+  set(PLATFORM_LIBS m c resolv)
 elseif(WIN32)
-    set(PLATFORM_LIBS ntdll ncrypt secur32 ws2_32 userenv bcrypt msvcrt advapi32 RuntimeObject)
+  set(PLATFORM_LIBS
+      ntdll
+      ncrypt
+      secur32
+      ws2_32
+      userenv
+      bcrypt
+      msvcrt
+      advapi32
+      RuntimeObject)
 else()
-    message(STATUS "UNKNOWN OS")
+  message(STATUS "UNKNOWN OS")
 endif()
 
-### Setup delta-kernel-rs dependency
+# Setup delta-kernel-rs dependency
 set(KERNEL_NAME delta_kernel)
 
 # Set default ExternalProject root directory
@@ -46,40 +59,50 @@ set(RUST_ENV_VARS "")
 # Propagate arch to rust build for CI
 set(RUST_PLATFORM_TARGET "")
 if("${OS_NAME}" STREQUAL "linux")
-    if ("${OS_ARCH}" STREQUAL "arm64")
-        set(RUST_PLATFORM_TARGET "aarch64-unknown-linux-gnu")
-    elseif("${CMAKE_CXX_COMPILER}" MATCHES "aarch64")
-        set(RUST_ENV_VARS ${RUST_ENV_VARS} CFLAGS_aarch64_unknown_linux_gnu=--sysroot=/usr/aarch64-linux-gnu)
-        set(RUST_ENV_VARS ${RUST_ENV_VARS} CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER=aarch64-linux-gnu-gcc)
-        set(RUST_ENV_VARS ${RUST_ENV_VARS} OPENSSL_LIB_DIR=${CMAKE_BINARY_DIR}/vcpkg_installed/${VCPKG_TARGET_TRIPLET}/lib)
-        set(RUST_ENV_VARS ${RUST_ENV_VARS} OPENSSL_INCLUDE_DIR=${CMAKE_BINARY_DIR}/vcpkg_installed/${VCPKG_TARGET_TRIPLET}/include)
-        set(RUST_PLATFORM_TARGET "aarch64-unknown-linux-gnu")
-    else()
-        set(RUST_PLATFORM_TARGET "x86_64-unknown-linux-gnu")
-    endif()
+  if("${OS_ARCH}" STREQUAL "arm64")
+    set(RUST_PLATFORM_TARGET "aarch64-unknown-linux-gnu")
+  elseif("${CMAKE_CXX_COMPILER}" MATCHES "aarch64")
+    set(RUST_ENV_VARS
+        ${RUST_ENV_VARS}
+        CFLAGS_aarch64_unknown_linux_gnu=--sysroot=/usr/aarch64-linux-gnu)
+    set(RUST_ENV_VARS
+        ${RUST_ENV_VARS}
+        CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER=aarch64-linux-gnu-gcc)
+    set(RUST_ENV_VARS
+        ${RUST_ENV_VARS}
+        OPENSSL_LIB_DIR=${CMAKE_BINARY_DIR}/vcpkg_installed/${VCPKG_TARGET_TRIPLET}/lib
+    )
+    set(RUST_ENV_VARS
+        ${RUST_ENV_VARS}
+        OPENSSL_INCLUDE_DIR=${CMAKE_BINARY_DIR}/vcpkg_installed/${VCPKG_TARGET_TRIPLET}/include
+    )
+    set(RUST_PLATFORM_TARGET "aarch64-unknown-linux-gnu")
+  else()
+    set(RUST_PLATFORM_TARGET "x86_64-unknown-linux-gnu")
+  endif()
 elseif("${OS_NAME}" STREQUAL "osx")
-    if ("${OSX_BUILD_ARCH}" STREQUAL "arm64")
-        set(RUST_PLATFORM_TARGET "aarch64-apple-darwin")
-    elseif ("${OSX_BUILD_ARCH}" STREQUAL "x86_64")
-        set(RUST_PLATFORM_TARGET "x86_64-apple-darwin")
-    elseif ("${OS_ARCH}" STREQUAL "arm64")
-        set(RUST_PLATFORM_TARGET "aarch64-apple-darwin")
-    endif()
+  if("${OSX_BUILD_ARCH}" STREQUAL "arm64")
+    set(RUST_PLATFORM_TARGET "aarch64-apple-darwin")
+  elseif("${OSX_BUILD_ARCH}" STREQUAL "x86_64")
+    set(RUST_PLATFORM_TARGET "x86_64-apple-darwin")
+  elseif("${OS_ARCH}" STREQUAL "arm64")
+    set(RUST_PLATFORM_TARGET "aarch64-apple-darwin")
+  endif()
 elseif(WIN32)
-    if (MINGW AND "${OS_ARCH}" STREQUAL "arm64")
-        set(RUST_PLATFORM_TARGET "aarch64-pc-windows-gnu")
-    elseif (MINGW AND "${OS_ARCH}" STREQUAL "amd64")
-        set(RUST_PLATFORM_TARGET "x86_64-pc-windows-gnu")
-    elseif (MSVC AND "${OS_ARCH}" STREQUAL "arm64")
-        set(RUST_PLATFORM_TARGET "aarch64-pc-windows-msvc")
-    elseif (MSVC AND "${OS_ARCH}" STREQUAL "amd64")
-        set(RUST_PLATFORM_TARGET "x86_64-pc-windows-msvc")
-    endif()
+  if(MINGW AND "${OS_ARCH}" STREQUAL "arm64")
+    set(RUST_PLATFORM_TARGET "aarch64-pc-windows-gnu")
+  elseif(MINGW AND "${OS_ARCH}" STREQUAL "amd64")
+    set(RUST_PLATFORM_TARGET "x86_64-pc-windows-gnu")
+  elseif(MSVC AND "${OS_ARCH}" STREQUAL "arm64")
+    set(RUST_PLATFORM_TARGET "aarch64-pc-windows-msvc")
+  elseif(MSVC AND "${OS_ARCH}" STREQUAL "amd64")
+    set(RUST_PLATFORM_TARGET "x86_64-pc-windows-msvc")
+  endif()
 endif()
 
 # We currently only support the predefined targets.
-if ("${RUST_PLATFORM_TARGET}" STREQUAL "")
-    message(FATAL_ERROR "Failed to detect the correct platform")
+if("${RUST_PLATFORM_TARGET}" STREQUAL "")
+  message(FATAL_ERROR "Failed to detect the correct platform")
 endif()
 
 set(RUST_PLATFORM_PARAM "--target=${RUST_PLATFORM_TARGET}")
@@ -92,69 +115,84 @@ string(STRIP "${RUST_ENV_VARS}" RUST_ENV_VARS)
 set(RUST_UNSET_ENV_VARS --unset=CC --unset=CXX --unset=LD)
 
 # Define all the relevant delta-kernel-rs paths/names
-set(DELTA_KERNEL_LIBNAME "${CMAKE_STATIC_LIBRARY_PREFIX}delta_kernel_ffi${CMAKE_STATIC_LIBRARY_SUFFIX}")
-set(DELTA_KERNEL_LIBPATH_DEBUG "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/${RUST_PLATFORM_TARGET}/debug/${DELTA_KERNEL_LIBNAME}")
-set(DELTA_KERNEL_LIBPATH_RELEASE "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/${RUST_PLATFORM_TARGET}/release/${DELTA_KERNEL_LIBNAME}")
-set(DELTA_KERNEL_FFI_HEADER_PATH "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/ffi-headers")
-set(DELTA_KERNEL_FFI_HEADER_C "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/ffi-headers/delta_kernel_ffi.h")
-set(DELTA_KERNEL_FFI_HEADER_CXX "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/ffi-headers/delta_kernel_ffi.hpp")
+set(DELTA_KERNEL_LIBNAME
+    "${CMAKE_STATIC_LIBRARY_PREFIX}delta_kernel_ffi${CMAKE_STATIC_LIBRARY_SUFFIX}"
+)
+set(DELTA_KERNEL_LIBPATH_DEBUG
+    "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/${RUST_PLATFORM_TARGET}/debug/${DELTA_KERNEL_LIBNAME}"
+)
+set(DELTA_KERNEL_LIBPATH_RELEASE
+    "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/${RUST_PLATFORM_TARGET}/release/${DELTA_KERNEL_LIBNAME}"
+)
+set(DELTA_KERNEL_FFI_HEADER_PATH
+    "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/ffi-headers")
+set(DELTA_KERNEL_FFI_HEADER_C
+    "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/ffi-headers/delta_kernel_ffi.h"
+)
+set(DELTA_KERNEL_FFI_HEADER_CXX
+    "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/ffi-headers/delta_kernel_ffi.hpp"
+)
 
 # Add rust_example as a CMake target
 ExternalProject_Add(
-        ${KERNEL_NAME}
-        GIT_REPOSITORY "https://github.com/delta-incubator/delta-kernel-rs"
-        # WARNING: the FFI headers are currently pinned due to the C linkage issue of the c++ headers. Currently, when bumping
-        # the kernel version, the produced header in ./src/include/delta_kernel_ffi.hpp should be also bumped, applying the fix
-        GIT_TAG v0.3.1
-        # Prints the env variables passed to the cargo build to the terminal, useful in debugging because passing them
-        # through CMake is an error-prone mess
-        CONFIGURE_COMMAND ${CMAKE_COMMAND} -E env ${RUST_UNSET_ENV_VARS} ${RUST_ENV_VARS} env
-        UPDATE_COMMAND ""
-        BUILD_IN_SOURCE 1
-        # Build debug build
-        BUILD_COMMAND
-            ${CMAKE_COMMAND} -E env ${RUST_UNSET_ENV_VARS} ${RUST_ENV_VARS}
-            cargo build --package delta_kernel_ffi --workspace --all-features ${RUST_PLATFORM_PARAM}
-        # Build release build
-        COMMAND
-            ${CMAKE_COMMAND} -E env ${RUST_UNSET_ENV_VARS} ${RUST_ENV_VARS}
-            cargo build --package delta_kernel_ffi --workspace --all-features --release ${RUST_PLATFORM_PARAM}
-        # Build DATs
-        COMMAND
-            ${CMAKE_COMMAND} -E env ${RUST_UNSET_ENV_VARS} ${RUST_ENV_VARS}
-            cargo build --manifest-path=${CMAKE_BINARY_DIR}/rust/src/delta_kernel/acceptance/Cargo.toml
-        # Define the byproducts, required for building with Ninja
-        BUILD_BYPRODUCTS "${DELTA_KERNEL_LIBPATH_DEBUG}"
-        BUILD_BYPRODUCTS "${DELTA_KERNEL_LIBPATH_RELEASE}"
-        BUILD_BYPRODUCTS "${DELTA_KERNEL_FFI_HEADER_C}"
-        BUILD_BYPRODUCTS "${DELTA_KERNEL_FFI_HEADER_CXX}"
-        INSTALL_COMMAND ""
-        LOG_BUILD ON)
+  ${KERNEL_NAME}
+  GIT_REPOSITORY "https://github.com/delta-incubator/delta-kernel-rs"
+  # WARNING: the FFI headers are currently pinned due to the C linkage issue of
+  # the c++ headers. Currently, when bumping the kernel version, the produced
+  # header in ./src/include/delta_kernel_ffi.hpp should be also bumped, applying
+  # the fix
+  GIT_TAG v0.4.0
+  # Prints the env variables passed to the cargo build to the terminal, useful
+  # in debugging because passing them through CMake is an error-prone mess
+  CONFIGURE_COMMAND ${CMAKE_COMMAND} -E env ${RUST_UNSET_ENV_VARS}
+                    ${RUST_ENV_VARS} env
+  UPDATE_COMMAND ""
+  BUILD_IN_SOURCE 1
+  # Build debug build
+  BUILD_COMMAND
+    ${CMAKE_COMMAND} -E env ${RUST_UNSET_ENV_VARS} ${RUST_ENV_VARS} cargo build
+    --package delta_kernel_ffi --workspace --all-features ${RUST_PLATFORM_PARAM}
+  # Build release build
+  COMMAND
+    ${CMAKE_COMMAND} -E env ${RUST_UNSET_ENV_VARS} ${RUST_ENV_VARS} cargo build
+    --package delta_kernel_ffi --workspace --all-features --release
+    ${RUST_PLATFORM_PARAM}
+  # Build DATs
+  COMMAND
+    ${CMAKE_COMMAND} -E env ${RUST_UNSET_ENV_VARS} ${RUST_ENV_VARS} cargo build
+    --manifest-path=${CMAKE_BINARY_DIR}/rust/src/delta_kernel/acceptance/Cargo.toml
+  # Define the byproducts, required for building with Ninja
+  BUILD_BYPRODUCTS "${DELTA_KERNEL_LIBPATH_DEBUG}"
+  BUILD_BYPRODUCTS "${DELTA_KERNEL_LIBPATH_RELEASE}"
+  BUILD_BYPRODUCTS "${DELTA_KERNEL_FFI_HEADER_C}"
+  BUILD_BYPRODUCTS "${DELTA_KERNEL_FFI_HEADER_CXX}"
+  INSTALL_COMMAND ""
+  LOG_BUILD ON)
 
 build_static_extension(${TARGET_NAME} ${EXTENSION_SOURCES})
 build_loadable_extension(${TARGET_NAME} " " ${EXTENSION_SOURCES})
 
-# TODO: when C linkage issue is resolved, we should switch back to using the generated headers
-#include_directories(${DELTA_KERNEL_FFI_HEADER_PATH})
+# TODO: when C linkage issue is resolved, we should switch back to using the
+# generated headers include_directories(${DELTA_KERNEL_FFI_HEADER_PATH})
 
 # Hides annoying linker warnings
-set(CMAKE_OSX_DEPLOYMENT_TARGET 13.3 CACHE STRING "Minimum OS X deployment version" FORCE)
+set(CMAKE_OSX_DEPLOYMENT_TARGET
+    13.3
+    CACHE STRING "Minimum OS X deployment version" FORCE)
 
 # Add the default client
 add_compile_definitions(DEFINE_DEFAULT_ENGINE)
 
 # Link delta-kernal-rs to static lib
-target_link_libraries(${EXTENSION_NAME}
-        debug ${DELTA_KERNEL_LIBPATH_DEBUG}
-        optimized ${DELTA_KERNEL_LIBPATH_RELEASE}
-        ${PLATFORM_LIBS})
+target_link_libraries(
+  ${EXTENSION_NAME} debug ${DELTA_KERNEL_LIBPATH_DEBUG} optimized
+  ${DELTA_KERNEL_LIBPATH_RELEASE} ${PLATFORM_LIBS})
 add_dependencies(${EXTENSION_NAME} delta_kernel)
 
 # Link delta-kernal-rs to dynamic lib
-target_link_libraries(${LOADABLE_EXTENSION_NAME}
-        debug ${DELTA_KERNEL_LIBPATH_DEBUG}
-        optimized ${DELTA_KERNEL_LIBPATH_RELEASE}
-        ${PLATFORM_LIBS})
+target_link_libraries(
+  ${LOADABLE_EXTENSION_NAME} debug ${DELTA_KERNEL_LIBPATH_DEBUG} optimized
+  ${DELTA_KERNEL_LIBPATH_RELEASE} ${PLATFORM_LIBS})
 add_dependencies(${LOADABLE_EXTENSION_NAME} delta_kernel)
 
 install(

src/delta_extension.cpp

@@ -1,8 +1,8 @@
 #define DUCKDB_EXTENSION_MAIN
 
 #include "delta_extension.hpp"
-#include "delta_functions.hpp"
 
+#include "delta_functions.hpp"
 #include "duckdb.hpp"
 #include "duckdb/common/exception.hpp"
 #include "duckdb/main/extension_util.hpp"
@@ -13,18 +13,18 @@
 namespace duckdb {
 
 static unique_ptr<Catalog> DeltaCatalogAttach(StorageExtensionInfo *storage_info, ClientContext &context,
-                                       AttachedDatabase &db, const string &name, AttachInfo &info,
-                                       AccessMode access_mode) {
+                                              AttachedDatabase &db, const string &name, AttachInfo &info,
+                                              AccessMode access_mode) {
 
-    auto res = make_uniq<DeltaCatalog>(db, info.path, access_mode);
+	auto res = make_uniq<DeltaCatalog>(db, info.path, access_mode);
 
-    for (const auto& option : info.options) {
-        if (StringUtil::Lower(option.first) == "pin_snapshot") {
-            res->use_cache = option.second.GetValue<bool>();
-        }
-    }
+	for (const auto &option : info.options) {
+		if (StringUtil::Lower(option.first) == "pin_snapshot") {
+			res->use_cache = option.second.GetValue<bool>();
+		}
+	}
 
-    res->SetDefaultTable(DEFAULT_SCHEMA, DEFAULT_DELTA_TABLE);
+	res->SetDefaultTable(DEFAULT_SCHEMA, DEFAULT_DELTA_TABLE);
 
 	return std::move(res);
 }
@@ -44,35 +44,35 @@ class DeltaStorageExtension : public StorageExtension {
 };
 
 static void LoadInternal(DatabaseInstance &instance) {
-    // Load functions
-    for (const auto &function : DeltaFunctions::GetTableFunctions(instance)) {
-        ExtensionUtil::RegisterFunction(instance, function);
-    }
-
-    // Register the "single table" delta catalog (to ATTACH a single delta table)
-    auto &config = DBConfig::GetConfig(instance);
-    config.storage_extensions["delta"] = make_uniq<DeltaStorageExtension>();
+	// Load functions
+	for (const auto &function : DeltaFunctions::GetTableFunctions(instance)) {
+		ExtensionUtil::RegisterFunction(instance, function);
+	}
+
+	// Register the "single table" delta catalog (to ATTACH a single delta table)
+	auto &config = DBConfig::GetConfig(instance);
+	config.storage_extensions["delta"] = make_uniq<DeltaStorageExtension>();
 }
 
 void DeltaExtension::Load(DuckDB &db) {
-    LoadInternal(*db.instance);
+	LoadInternal(*db.instance);
 }
 
 std::string DeltaExtension::Name() {
-    return "delta";
+	return "delta";
 }
 
 } // namespace duckdb
 
 extern "C" {
 
 DUCKDB_EXTENSION_API void delta_init(duckdb::DatabaseInstance &db) {
-    duckdb::DuckDB db_wrapper(db);
-    db_wrapper.LoadExtension<duckdb::DeltaExtension>();
+	duckdb::DuckDB db_wrapper(db);
+	db_wrapper.LoadExtension<duckdb::DeltaExtension>();
 }
 
 DUCKDB_EXTENSION_API const char *delta_version() {
-    return duckdb::DuckDB::LibraryVersion();
+	return duckdb::DuckDB::LibraryVersion();
 }
 }