Merge branch 'feature' into add-expression-test

samansmink · Dec 19, 2024 · da26428 · da26428
2 parents 9876698 + fd9dc3e
commit da26428
Show file tree

Hide file tree

Showing 11 changed files with 735 additions and 404 deletions.
diff --git a/.github/workflows/LocalTesting.yml b/.github/workflows/LocalTesting.yml
@@ -294,11 +294,12 @@ jobs:
         run: |
           python ./duckdb/scripts/regression/test_runner.py --old=duckdb_delta/build/release/benchmark/benchmark_runner --new=build/release/benchmark/benchmark_runner --benchmarks=.github/regression/tpcds_sf1_local.csv --verbose --threads=2 --root-dir=.
 
-      - name: Regression Test Micro
-        if: always()
-        shell: bash
-        run: |
-          python ./duckdb/scripts/regression/test_runner.py --old=duckdb_delta/build/release/benchmark/benchmark_runner --new=build/release/benchmark/benchmark_runner --benchmarks=.github/regression/micro.csv --verbose --threads=2 --root-dir=.
+      # FIXME: re-enable
+#      - name: Regression Test Micro
+#        if: always()
+#        shell: bash
+#        run: |
+#          python ./duckdb/scripts/regression/test_runner.py --old=duckdb_delta/build/release/benchmark/benchmark_runner --new=build/release/benchmark/benchmark_runner --benchmarks=.github/regression/micro.csv --verbose --threads=2 --root-dir=.
 
       - name: Test benchmark makefile
         shell: bash

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -37,6 +37,7 @@ elseif(UNIX)
 elseif(WIN32)
   set(PLATFORM_LIBS
       ntdll
+      crypt32
       ncrypt
       secur32
       ws2_32
@@ -119,11 +120,8 @@ set(RUST_UNSET_ENV_VARS --unset=CC --unset=CXX --unset=LD)
 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_LIBPATH
+    "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/${RUST_PLATFORM_TARGET}/$<IF:$<CONFIG:Debug>,debug,release>/${DELTA_KERNEL_LIBNAME}"
 )
 set(DELTA_KERNEL_FFI_HEADER_PATH
     "${CMAKE_BINARY_DIR}/rust/src/delta_kernel/target/ffi-headers")
@@ -142,7 +140,7 @@ ExternalProject_Add(
   # 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.5.0
+  GIT_TAG v0.6.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}
@@ -152,19 +150,13 @@ ExternalProject_Add(
   # 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}
+    --package delta_kernel_ffi --workspace  $<$<CONFIG:Release>:--release> --all-features ${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_LIBPATH}"
   BUILD_BYPRODUCTS "${DELTA_KERNEL_FFI_HEADER_C}"
   BUILD_BYPRODUCTS "${DELTA_KERNEL_FFI_HEADER_CXX}"
   INSTALL_COMMAND ""
@@ -186,14 +178,12 @@ 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})
+  ${EXTENSION_NAME} ${DELTA_KERNEL_LIBPATH} ${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})
+  ${LOADABLE_EXTENSION_NAME} ${DELTA_KERNEL_LIBPATH} ${PLATFORM_LIBS})
 add_dependencies(${LOADABLE_EXTENSION_NAME} delta_kernel)
 
 install(

diff --git a/Makefile b/Makefile
@@ -4,6 +4,14 @@ PROJ_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
 EXT_NAME=deltatable
 EXT_CONFIG=${PROJ_DIR}extension_config.cmake
 
+ifeq ($(SANITIZER_MODE), thread)
+	EXT_DEBUG_FLAGS:=-DENABLE_THREAD_SANITIZER=1
+endif
+
+ifneq ("${CUSTOM_LINKER}", "")
+	EXT_DEBUG_FLAGS:=${EXT_DEBUG_FLAGS} -DCUSTOM_LINKER=${CUSTOM_LINKER}
+endif
+
 # Set test paths
 test_release: export DELTA_KERNEL_TESTS_PATH=./build/release/rust/src/delta_kernel/kernel/tests/data
 test_release: export DAT_PATH=./build/release/rust/src/delta_kernel/acceptance/tests/dat
@@ -12,7 +20,7 @@ test_debug: export DELTA_KERNEL_TESTS_PATH=./build/debug/rust/src/delta_kernel/k
 test_debug: export DAT_PATH=./build/debug/rust/src/delta_kernel/acceptance/tests/dat
 
 # Core extensions that we need for testing
-CORE_EXTENSIONS='tpcds;tpch;aws;azure;httpfs'
+#CORE_EXTENSIONS='tpcds;tpch;aws;azure;httpfs'
 
 # Set this flag during building to enable the benchmark runner
 ifeq (${BUILD_BENCHMARK}, 1)

diff --git a/src/delta_utils.cpp b/src/delta_utils.cpp
@@ -372,18 +372,15 @@ uintptr_t SchemaVisitor::MakeFieldListImpl(uintptr_t capacity_hint) {
 void SchemaVisitor::AppendToList(uintptr_t id, ffi::KernelStringSlice name, LogicalType &&child) {
 	auto it = inflight_lists.find(id);
 	if (it == inflight_lists.end()) {
-		// TODO... some error...
-		throw InternalException("WEIRD SHIT");
-	} else {
-		it->second->emplace_back(std::make_pair(string(name.ptr, name.len), std::move(child)));
+		throw InternalException("Unhandled error in SchemaVisitor::AppendToList child");
 	}
+	it->second->emplace_back(std::make_pair(string(name.ptr, name.len), std::move(child)));
 }
 
 unique_ptr<SchemaVisitor::FieldList> SchemaVisitor::TakeFieldList(uintptr_t id) {
 	auto it = inflight_lists.find(id);
 	if (it == inflight_lists.end()) {
-		// TODO: Raise some kind of error.
-		throw InternalException("WEIRD SHIT 2");
+		throw InternalException("Unhandled error in SchemaVisitor::TakeFieldList");
 	}
 	auto rval = std::move(it->second);
 	inflight_lists.erase(it);
@@ -438,10 +435,12 @@ string DuckDBEngineError::KernelErrorEnumToString(ffi::KernelError err) {
         "MissingCommitInfo",
         "UnsupportedError",
         "ParseIntervalError",
-        "ChangeDataFeedUnsupported"
+        "ChangeDataFeedUnsupported",
+	    "ChangeDataFeedIncompatibleSchema",
+        "InvalidCheckpoint"
 	};
 
-	static_assert(sizeof(KERNEL_ERROR_ENUM_STRINGS) / sizeof(char *) - 1 == (int)ffi::KernelError::ChangeDataFeedUnsupported,
+	static_assert(sizeof(KERNEL_ERROR_ENUM_STRINGS) / sizeof(char *) - 1 == (int)ffi::KernelError::InvalidCheckpoint,
 	              "KernelErrorEnumStrings mismatched with kernel");
 
 	if ((int)err < sizeof(KERNEL_ERROR_ENUM_STRINGS) / sizeof(char *)) {