Fix extension bugs

kuzudb · Apr 24, 2024 · ab6e496 · ab6e496
1 parent b4b7dc6
commit ab6e496
Show file tree

Hide file tree

Showing 9 changed files with 72 additions and 160 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -217,7 +217,7 @@ add_subdirectory(third_party)
 if(${BUILD_KUZU})
 add_definitions(-DKUZU_ROOT_DIRECTORY="${PROJECT_SOURCE_DIR}")
 add_definitions(-DKUZU_CMAKE_VERSION="${CMAKE_PROJECT_VERSION}")
-add_definitions(-DKUZU_EXTENSION_VERSION="0.2.6")
+add_definitions(-DKUZU_EXTENSION_VERSION="0.2.7")
 
 include_directories(src/include)
 

diff --git a/extension/duckdb_scanner/src/duckdb_catalog.cpp b/extension/duckdb_scanner/src/duckdb_catalog.cpp
@@ -1,6 +1,7 @@
 #include "duckdb_catalog.h"
 
 #include "common/exception/binder.h"
+#include "common/exception/runtime.h"
 #include "duckdb_scan.h"
 #include "duckdb_table_catalog_entry.h"
 #include "duckdb_type_converter.h"
@@ -22,7 +23,7 @@ void DuckDBCatalogContent::init(const std::string& dbPath, const std::string& ca
     } catch (std::exception& e) {
         throw common::BinderException(e.what());
     }
-    if (resultChunk->size() == 0) {
+    if (resultChunk == nullptr || resultChunk->size() == 0) {
         return;
     }
     common::ValueVector tableNamesVector{*common::LogicalType::STRING(),
@@ -126,9 +127,13 @@ std::string DuckDBCatalogContent::getDefaultSchemaName() const {
 
 std::pair<duckdb::DuckDB, duckdb::Connection> DuckDBCatalogContent::getConnection(
     const std::string& dbPath) const {
-    duckdb::DuckDB db(dbPath);
-    duckdb::Connection con(db);
-    return std::make_pair(std::move(db), std::move(con));
+    try {
+        duckdb::DuckDB db(dbPath);
+        duckdb::Connection con(db);
+        return std::make_pair(std::move(db), std::move(con));
+    } catch (std::exception& e) {
+        throw common::RuntimeException{e.what()};
+    }
 }
 
 } // namespace duckdb_scanner

diff --git a/extension/duckdb_scanner/src/duckdb_storage.cpp b/extension/duckdb_scanner/src/duckdb_storage.cpp
@@ -2,6 +2,8 @@
 
 #include <filesystem>
 
+#include "common/exception/runtime.h"
+#include "common/file_system/virtual_file_system.h"
 #include "common/string_utils.h"
 #include "duckdb_catalog.h"
 
@@ -20,6 +22,11 @@ std::unique_ptr<main::AttachedDatabase> attachDuckDB(std::string dbName, std::st
         dbName = catalogName;
     }
     auto duckdbCatalog = std::make_unique<DuckDBCatalogContent>();
+    auto vfs = std::make_unique<common::VirtualFileSystem>();
+    if (!vfs->fileOrPathExists(dbPath)) {
+        throw common::RuntimeException{
+            common::stringFormat("'{}' is not a valid duckdb database path.", dbPath)};
+    }
     duckdbCatalog->init(dbPath, catalogName, clientContext);
     return std::make_unique<main::AttachedDatabase>(dbName, std::move(duckdbCatalog));
 }

diff --git a/extension/duckdb_scanner/test/test_files/duckdb_scanner.test b/extension/duckdb_scanner/test/test_files/duckdb_scanner.test
@@ -8,7 +8,7 @@
 ---- ok
 -STATEMENT ATTACH '${KUZU_ROOT_DIRECTORY}/extension/duckdb_scanner/test/duckdb_database/tinysnb.db' as tinysnb (dbtype 'duckdb');
 ---- 1
-Attach database successfully.
+Attached database successfully.
 -STATEMENT LOAD FROM tinysnb.person RETURN *;
 ---- 8
 0|Alice|1|True|False|35|5.000000|1900-01-01|2011-08-20 11:25:30|3 years 2 days 13:02:00|[10,5]|[Aida]|[[10,8],[6,7,8]]|1.731000|a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11|[96,54,86,92]
@@ -41,17 +41,17 @@ The 😂😃🧘🏻‍♂️🌍🌦️🍞🚗 movie|2544| the movie is very v
 Catalog exception: Table: person1 does not exist.
 -STATEMENT DETACH tinysnb;
 ---- 1
-Detach database successfully.
+Detached database successfully.
 -STATEMENT LOAD FROM tinysnb.person RETURN *;
 ---- error
 Binder exception: No database named tinysnb has been attached.
 -LOG AttachMultipleDuckDB
 -STATEMENT ATTACH '${KUZU_ROOT_DIRECTORY}/extension/duckdb_scanner/test/duckdb_database/tinysnb.db' (dbtype 'duckdb');
 ---- 1
-Attach database successfully.
+Attached database successfully.
 -STATEMENT ATTACH '${KUZU_ROOT_DIRECTORY}/extension/duckdb_scanner/test/duckdb_database/other.db' as Other1 (dbtype 'duckdb');
 ---- 1
-Attach database successfully.
+Attached database successfully.
 -STATEMENT LOAD FROM other1.person RETURN *;
 ---- 4
 1
@@ -64,7 +64,7 @@ Attach database successfully.
 -LOG UseDatabaseStatement
 -STATEMENT USE other1;
 ---- 1
-Use database successfully.
+Used database successfully.
 -STATEMENT LOAD FROM person RETURN *;
 ---- 4
 1
@@ -73,7 +73,7 @@ Use database successfully.
 5
 -STATEMENT USE tinysnb;
 ---- 1
-Use database successfully.
+Used database successfully.
 -STATEMENT LOAD FROM person RETURN count(*);
 ---- 1
 8
@@ -107,3 +107,29 @@ Wendy|28
 -STATEMENT ATTACH '${KUZU_ROOT_DIRECTORY}/extension/duckdb_scanner/test/duckdb_database/dbfilewithoutext' (dbtype 'duckdb');
 ---- error
 Runtime exception: Database with name: dbfilewithoutext has already been attached.
+-LOG DetachExistDB
+-STATEMENT DETACH dbfilewithoutext;
+---- ok
+-LOG DetachNotExistDBError
+-STATEMENT DETACH dbfilewithoutext;
+---- error
+Runtime exception: Database: dbfilewithoutext doesn't exist.
+-LOG AttachNotExistDBError
+-STATEMENT ATTACH 'notexist.db' (dbtype 'duckdb');
+---- error
+Runtime exception: 'notexist.db' is not a valid duckdb database path.
+
+-CASE AttachDBWithoutLoadingExtension
+-STATEMENT ATTACH 'notexist.db' (dbtype 'duckdb');
+---- error
+Runtime exception: No loaded extension can handle database type: duckdb.
+Did you forget to load duckdb extension?
+You can load it by: load extension duckdb_scanner;
+-STATEMENT ATTACH 'notexist.db' (dbtype 'postgres');
+---- error
+Runtime exception: No loaded extension can handle database type: postgres.
+Did you forget to load postgres extension?
+You can load it by: load extension postgres_scanner;
+-STATEMENT ATTACH 'notexist.db' (dbtype 'sqlite');
+---- error
+Runtime exception: No loaded extension can handle database type: sqlite.
diff --git a/src/function/list/list_extract_function.cpp b/src/function/list/list_extract_function.cpp
@@ -20,80 +20,10 @@ static std::unique_ptr<FunctionBindData> ListExtractBindFunc(
     const binder::expression_vector& arguments, Function* function) {
     auto resultType = ListType::getChildType(&arguments[0]->dataType);
     auto scalarFunction = ku_dynamic_cast<Function*, ScalarFunction*>(function);
-    switch (resultType->getPhysicalType()) {
-    case PhysicalTypeID::BOOL: {
+    TypeUtils::visit(resultType->getPhysicalType(), [&]<typename T>(T) {
         scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, uint8_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::INT64: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, int64_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::INT32: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, int32_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::INT16: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, int16_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::INT8: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, int8_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::UINT64: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, uint64_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::UINT32: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, uint32_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::UINT16: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, uint16_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::UINT8: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, uint8_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::INT128: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, int128_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::DOUBLE: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, double, ListExtract>;
-    } break;
-    case PhysicalTypeID::FLOAT: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, float, ListExtract>;
-    } break;
-    case PhysicalTypeID::INTERVAL: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, interval_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::STRING: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, ku_string_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::ARRAY:
-    case PhysicalTypeID::LIST: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, list_entry_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::STRUCT: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, struct_entry_t, ListExtract>;
-    } break;
-    case PhysicalTypeID::INTERNAL_ID: {
-        scalarFunction->execFunc =
-            BinaryExecListExtractFunction<list_entry_t, int64_t, internalID_t, ListExtract>;
-    } break;
-    default: {
-        KU_UNREACHABLE;
-    }
-    }
+            BinaryExecListExtractFunction<list_entry_t, int64_t, T, ListExtract>;
+    });
     std::vector<LogicalType> paramTypes;
     paramTypes.push_back(arguments[0]->getDataType());
     paramTypes.push_back(LogicalType(function->parameterTypeIDs[1]));

diff --git a/src/function/map/map_extract_function.cpp b/src/function/map/map_extract_function.cpp
@@ -1,6 +1,7 @@
 #include "function/map/functions/map_extract_function.h"
 
 #include "common/exception/runtime.h"
+#include "common/type_utils.h"
 #include "function/map/vector_map_functions.h"
 #include "function/scalar_function.h"
 
@@ -21,80 +22,10 @@ static std::unique_ptr<FunctionBindData> bindFunc(const binder::expression_vecto
     kuzu::function::Function* function) {
     validateKeyType(arguments[0], arguments[1]);
     auto scalarFunction = ku_dynamic_cast<Function*, ScalarFunction*>(function);
-    switch (arguments[1]->getDataType().getPhysicalType()) {
-    case PhysicalTypeID::BOOL: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            uint8_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::INT64: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            int64_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::INT32: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            int32_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::INT16: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            int16_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::INT8: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            int8_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::UINT64: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            uint64_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::UINT32: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            uint32_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::UINT16: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            uint16_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::UINT8: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            uint8_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::INT128: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            int128_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::DOUBLE: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            double, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::FLOAT: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t, float,
-            list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::STRING: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            ku_string_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::INTERVAL: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            interval_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::INTERNAL_ID: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            internalID_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::ARRAY:
-    case PhysicalTypeID::LIST: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            list_entry_t, list_entry_t, MapExtract>;
-    } break;
-    case PhysicalTypeID::STRUCT: {
-        scalarFunction->execFunc = ScalarFunction::BinaryExecListStructFunction<list_entry_t,
-            struct_entry_t, list_entry_t, MapExtract>;
-    } break;
-    default: {
-        KU_UNREACHABLE;
-    }
-    }
+    TypeUtils::visit(arguments[1]->getDataType().getPhysicalType(), [&]<typename T>(T) {
+        scalarFunction->execFunc =
+            ScalarFunction::BinaryExecListStructFunction<list_entry_t, T, list_entry_t, MapExtract>;
+    });
     auto resultType = LogicalType::LIST(MapType::getValueType(&arguments[0]->dataType)->copy());
     return FunctionBindData::getSimpleBindData(arguments, *resultType);
 }

diff --git a/src/include/common/file_system/virtual_file_system.h b/src/include/common/file_system/virtual_file_system.h
@@ -8,7 +8,7 @@
 namespace kuzu {
 namespace common {
 
-class VirtualFileSystem final : public FileSystem {
+class KUZU_API VirtualFileSystem final : public FileSystem {
 
 public:
     VirtualFileSystem();

diff --git a/src/main/database_manager.cpp b/src/main/database_manager.cpp
@@ -41,7 +41,8 @@ void DatabaseManager::detachDatabase(const std::string& databaseName) {
             return;
         }
     }
-    KU_UNREACHABLE;
+    throw common::RuntimeException{
+        common::stringFormat("Database: {} doesn't exist.", databaseName)};
 }
 
 void DatabaseManager::setDefaultDatabase(const std::string& databaseName) {

diff --git a/src/processor/operator/simple/attach_database.cpp b/src/processor/operator/simple/attach_database.cpp
@@ -1,5 +1,6 @@
 #include "processor/operator/simple/attach_database.h"
 
+#include "common/exception/runtime.h"
 #include "main/database.h"
 #include "main/database_manager.h"
 #include "storage/storage_extension.h"
@@ -13,8 +14,19 @@ void AttachDatabase::executeInternal(ExecutionContext* context) {
         if (storageExtension->canHandleDB(attachInfo.dbType)) {
             auto db = storageExtension->attach(attachInfo.dbAlias, attachInfo.dbPath, client);
             client->getDatabaseManager()->registerAttachedDatabase(std::move(db));
+            return;
         }
     }
+    auto errMsg = common::stringFormat("No loaded extension can handle database type: {}.",
+        attachInfo.dbType);
+    if (attachInfo.dbType == "duckdb") {
+        errMsg += "\nDid you forget to load duckdb extension?\nYou can load it by: load "
+                  "extension duckdb_scanner;";
+    } else if (attachInfo.dbType == "postgres") {
+        errMsg += "\nDid you forget to load postgres extension?\nYou can load it by: load "
+                  "extension postgres_scanner;";
+    }
+    throw common::RuntimeException{errMsg};
 }
 
 std::string AttachDatabase::getOutputMsg() {