Fix ice-disk table scans

docs/icebug-disk.md

@@ -25,7 +25,7 @@ CREATE REL TABLE livesin(FROM user TO city) WITH (storage = '<path-to-dir>', for
 
 File paths can be relative or absolute and are resolved as `<path-to-dir>/nodes_{tableName}.parquet` for node tables, and `<path-to-dir>/indices_{tableName}.parquet` and `<path-to-dir>/indptr_{tableName}.parquet` for relationship tables.
 
-Object-store URIs (e.g. `s3://bucket/path`, `https://host/path`) are supported as `storage` values and are passed through unchanged.
+Object-store URIs (e.g. `s3://bucket/path`, `https://host/path`) are also supported as `storage` values.
 
 If `storage` is omitted when `format = 'icebug-disk'` is set, files are resolved relative to the current working directory.
 

src/binder/bind/bind_ddl.cpp

@@ -15,6 +15,7 @@
 #include "catalog/catalog_entry/sequence_catalog_entry.h"
 #include "common/constants.h"
 #include "common/enums/extend_direction_util.h"
+#include "common/enums/storage_format.h"
 #include "common/exception/binder.h"
 #include "common/exception/message.h"
 #include "common/string_utils.h"
@@ -203,12 +204,12 @@ static ExtendDirection getStorageDirection(const case_insensitive_map_t<Value>&
     return DEFAULT_EXTEND_DIRECTION;
 }
 
-static std::string getStorageFormat(const case_insensitive_map_t<Value>& options) {
+static StorageFormat getStorageFormat(const case_insensitive_map_t<Value>& options) {
     if (options.contains(TableOptionConstants::STORAGE_FORMAT_OPTION)) {
-        return options.at(TableOptionConstants::STORAGE_FORMAT_OPTION).toString();
+        return StorageFormatUtils::fromString(
+            options.at(TableOptionConstants::STORAGE_FORMAT_OPTION).toString());
     }
-
-    return "";
+    return StorageFormat::NONE;
 }
 
 BoundCreateTableInfo Binder::bindCreateNodeTableInfo(const CreateTableInfo* info) {
@@ -252,8 +253,7 @@ BoundCreateTableInfo Binder::bindCreateRelTableGroupInfo(const CreateTableInfo*
         // Handle special case where icebug-disk storage could contain a dot
         // Otherwise, treat as file path (e.g., "dataset/demo-db/icebug-disk/demo" or
         // "data.parquet")
-        if (!TableOptionConstants::isIceBugDiskFormat(storageFormat) &&
-            dotPos != std::string::npos) {
+        if (storageFormat != StorageFormat::ICEBUG_DISK && dotPos != std::string::npos) {
             std::string dbName = storage.substr(0, dotPos);
             std::string tableName = storage.substr(dotPos + 1);
             if (!dbName.empty()) {
@@ -344,17 +344,15 @@ BoundCreateTableInfo Binder::bindCreateRelTableGroupInfo(const CreateTableInfo*
             }
         }
 
-        bool isSrcIcebugDisk =
-            srcEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ?
-                TableOptionConstants::isIceBugDiskFormat(
-                    srcEntry->ptrCast<NodeTableCatalogEntry>()->getStorageFormat()) :
-                false;
-        bool isDstIcebugDisk =
-            dstEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ?
-                TableOptionConstants::isIceBugDiskFormat(
-                    dstEntry->ptrCast<NodeTableCatalogEntry>()->getStorageFormat()) :
-                false;
-        bool isRelIcebugDisk = TableOptionConstants::isIceBugDiskFormat(storageFormat);
+        bool isSrcIcebugDisk = srcEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ?
+                                   srcEntry->ptrCast<NodeTableCatalogEntry>()->getStorageFormat() ==
+                                       StorageFormat::ICEBUG_DISK :
+                                   false;
+        bool isDstIcebugDisk = dstEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ?
+                                   dstEntry->ptrCast<NodeTableCatalogEntry>()->getStorageFormat() ==
+                                       StorageFormat::ICEBUG_DISK :
+                                   false;
+        bool isRelIcebugDisk = (storageFormat == StorageFormat::ICEBUG_DISK);
 
         // We don't allow mixing icebug-disk tables with non-icebug-disk tables
         // We only allow icebug-disk rel tables to connect icebug-disk node tables
@@ -609,15 +607,15 @@ static void validateNotIceDiskTable(main::ClientContext* clientContext,
     }
 
     auto tableEntry = catalog->getTableCatalogEntry(transaction, tableName);
-    std::string storageFormat;
+    StorageFormat storageFormat = StorageFormat::NONE;
 
     if (tableEntry->getTableType() == common::TableType::NODE) {
         storageFormat = tableEntry->ptrCast<NodeTableCatalogEntry>()->getStorageFormat();
     } else if (tableEntry->getTableType() == common::TableType::REL) {
         storageFormat = tableEntry->ptrCast<RelGroupCatalogEntry>()->getStorageFormat();
     }
 
-    if (TableOptionConstants::isIceBugDiskFormat(storageFormat)) {
+    if (storageFormat == StorageFormat::ICEBUG_DISK) {
         throw BinderException(
             std::format("Cannot alter table {}: icebug-disk tables are immutable.", tableName));
     }

src/catalog/catalog_entry/node_table_catalog_entry.cpp

@@ -2,50 +2,26 @@
 
 #include "binder/ddl/bound_create_table_info.h"
 #include "common/constants.h"
-#include "common/serializer/buffered_file.h"
+#include "common/enums/storage_format.h"
 #include "common/serializer/deserializer.h"
 #include "common/string_utils.h"
 #include "storage/storage_version_info.h"
 #include <format>
 
 using namespace lbug::binder;
+using namespace lbug::common;
 
 namespace lbug {
 namespace catalog {
 
-static void upgradeLegacyStorageFormat(const std::string& storage, std::string& storageFormat) {
+static void upgradeLegacyStorageFormat(const std::string& storage,
+    common::StorageFormat& storageFormat) {
     const auto lowerStorage = common::StringUtils::getLower(storage);
     if (lowerStorage.ends_with("parquet")) {
-        storageFormat = std::string(common::TableOptionConstants::ICEBUG_DISK_FORMAT);
+        storageFormat = common::StorageFormat::ICEBUG_DISK;
     }
 }
 
-static bool tryDeserializeStorageFormat(common::Deserializer& deserializer,
-    std::string& storageFormat) {
-    auto* reader = dynamic_cast<common::BufferedFileReader*>(deserializer.getReader());
-    if (reader == nullptr) {
-        deserializer.deserializeValue(storageFormat);
-        return true;
-    }
-    const auto readOffset = reader->getReadOffset();
-    uint64_t valueLength = 0;
-    deserializer.deserializeValue(valueLength);
-    constexpr uint64_t MAX_STORAGE_FORMAT_LENGTH = 1024;
-    if (valueLength > MAX_STORAGE_FORMAT_LENGTH) {
-        reader->resetReadOffset(readOffset);
-        return false;
-    }
-    storageFormat.resize(valueLength);
-    deserializer.read(reinterpret_cast<uint8_t*>(storageFormat.data()), valueLength);
-    if (!storageFormat.empty() &&
-        !common::TableOptionConstants::isIceBugDiskFormat(storageFormat)) {
-        reader->resetReadOffset(readOffset);
-        storageFormat.clear();
-        return false;
-    }
-    return true;
-}
-
 void NodeTableCatalogEntry::renameProperty(const std::string& propertyName,
     const std::string& newName) {
     TableCatalogEntry::renameProperty(propertyName, newName);
@@ -61,25 +37,23 @@ void NodeTableCatalogEntry::serialize(common::Serializer& serializer) const {
     serializer.writeDebuggingInfo("storage");
     serializer.write(storage);
     serializer.writeDebuggingInfo("storageFormat");
-    serializer.write(storageFormat);
+    serializer.serializeValue(storageFormat);
 }
 
 std::unique_ptr<NodeTableCatalogEntry> NodeTableCatalogEntry::deserialize(
     common::Deserializer& deserializer) {
     std::string debuggingInfo;
     std::string primaryKeyName;
     std::string storage;
-    std::string storageFormat;
+    auto storageFormat = StorageFormat::NONE;
     deserializer.validateDebuggingInfo(debuggingInfo, "primaryKeyName");
     deserializer.deserializeValue(primaryKeyName);
     deserializer.validateDebuggingInfo(debuggingInfo, "storage");
     deserializer.deserializeValue(storage);
     if (deserializer.getStorageVersion() >=
         ::lbug::storage::StorageVersionInfo::STORAGE_VERSION_41) {
         deserializer.validateDebuggingInfo(debuggingInfo, "storageFormat");
-        if (!tryDeserializeStorageFormat(deserializer, storageFormat)) {
-            upgradeLegacyStorageFormat(storage, storageFormat);
-        }
+        deserializer.deserializeValue(storageFormat);
     } else {
         upgradeLegacyStorageFormat(storage, storageFormat);
     }

src/catalog/catalog_entry/rel_group_catalog_entry.cpp

@@ -5,7 +5,7 @@
 #include "binder/ddl/bound_create_table_info.h"
 #include "catalog/catalog.h"
 #include "common/constants.h"
-#include "common/serializer/buffered_file.h"
+#include "common/enums/storage_format.h"
 #include "common/serializer/deserializer.h"
 #include "common/string_utils.h"
 #include "storage/storage_version_info.h"
@@ -18,38 +18,14 @@ using namespace lbug::main;
 namespace lbug {
 namespace catalog {
 
-static void upgradeLegacyStorageFormat(const std::string& storage, std::string& storageFormat) {
+static void upgradeLegacyStorageFormat(const std::string& storage,
+    common::StorageFormat& storageFormat) {
     const auto lowerStorage = common::StringUtils::getLower(storage);
     if (lowerStorage.ends_with("parquet")) {
-        storageFormat = std::string(common::TableOptionConstants::ICEBUG_DISK_FORMAT);
+        storageFormat = common::StorageFormat::ICEBUG_DISK;
     }
 }
 
-static bool tryDeserializeStorageFormat(Deserializer& deserializer, std::string& storageFormat) {
-    auto* reader = dynamic_cast<common::BufferedFileReader*>(deserializer.getReader());
-    if (reader == nullptr) {
-        deserializer.deserializeValue(storageFormat);
-        return true;
-    }
-    const auto readOffset = reader->getReadOffset();
-    uint64_t valueLength = 0;
-    deserializer.deserializeValue(valueLength);
-    constexpr uint64_t MAX_STORAGE_FORMAT_LENGTH = 1024;
-    if (valueLength > MAX_STORAGE_FORMAT_LENGTH) {
-        reader->resetReadOffset(readOffset);
-        return false;
-    }
-    storageFormat.resize(valueLength);
-    deserializer.read(reinterpret_cast<uint8_t*>(storageFormat.data()), valueLength);
-    if (!storageFormat.empty() &&
-        !common::TableOptionConstants::isIceBugDiskFormat(storageFormat)) {
-        reader->resetReadOffset(readOffset);
-        storageFormat.clear();
-        return false;
-    }
-    return true;
-}
-
 void RelGroupCatalogEntry::addFromToConnection(table_id_t srcTableID, table_id_t dstTableID,
     oid_t oid) {
     relTableInfos.emplace_back(NodeTableIDPair{srcTableID, dstTableID}, oid);
@@ -152,7 +128,7 @@ std::unique_ptr<RelGroupCatalogEntry> RelGroupCatalogEntry::deserialize(
     auto dstMultiplicity = RelMultiplicity::MANY;
     auto storageDirection = ExtendDirection::BOTH;
     std::string storage;
-    std::string storageFormat;
+    auto storageFormat = StorageFormat::NONE;
     std::vector<RelTableCatalogInfo> relTableInfos;
     deserializer.validateDebuggingInfo(debuggingInfo, "srcMultiplicity");
     deserializer.deserializeValue(srcMultiplicity);
@@ -174,9 +150,7 @@ std::unique_ptr<RelGroupCatalogEntry> RelGroupCatalogEntry::deserialize(
     if (deserializer.getStorageVersion() >=
         ::lbug::storage::StorageVersionInfo::STORAGE_VERSION_41) {
         deserializer.validateDebuggingInfo(debuggingInfo, "storageFormat");
-        if (!tryDeserializeStorageFormat(deserializer, storageFormat)) {
-            upgradeLegacyStorageFormat(storage, storageFormat);
-        }
+        deserializer.deserializeValue(storageFormat);
     } else {
         upgradeLegacyStorageFormat(storage, storageFormat);
     }

src/common/enums/CMakeLists.txt

@@ -1,6 +1,7 @@
 add_library(lbug_common_enums
         OBJECT
         accumulate_type.cpp
+        storage_format.cpp
         path_semantic.cpp
         query_rel_type.cpp
         rel_direction.cpp

src/common/enums/storage_format.cpp

@@ -0,0 +1,18 @@
+#include "common/enums/storage_format.h"
+
+#include "common/exception/binder.h"
+#include <format>
+
+namespace lbug {
+namespace common {
+
+StorageFormat StorageFormatUtils::fromString(const std::string& str) {
+    if (str == "icebug-disk") {
+        return StorageFormat::ICEBUG_DISK;
+    }
+    throw BinderException(
+        std::format("Unsupported storage format '{}'. Valid options are: icebug-disk.", str));
+}
+
+} // namespace common
+} // namespace lbug

src/include/binder/ddl/bound_create_table_info.h

@@ -7,6 +7,7 @@
 #include "common/enums/conflict_action.h"
 #include "common/enums/extend_direction.h"
 #include "common/enums/rel_multiplicity.h"
+#include "common/enums/storage_format.h"
 #include "function/table/bind_data.h"
 #include "function/table/table_function.h"
 #include "property_definition.h"
@@ -76,14 +77,14 @@ struct LBUG_API BoundExtraCreateTableInfo : BoundExtraCreateCatalogEntryInfo {
 struct BoundExtraCreateNodeTableInfo final : BoundExtraCreateTableInfo {
     std::string primaryKeyName;
     std::string storage;
-    std::string storageFormat;
+    common::StorageFormat storageFormat = common::StorageFormat::NONE;
 
     BoundExtraCreateNodeTableInfo(std::string primaryKeyName,
         std::vector<PropertyDefinition> definitions, std::string storage = "",
-        std::string storageFormat = "")
+        common::StorageFormat storageFormat = common::StorageFormat::NONE)
         : BoundExtraCreateTableInfo{std::move(definitions)},
           primaryKeyName{std::move(primaryKeyName)}, storage{std::move(storage)},
-          storageFormat{std::move(storageFormat)} {}
+          storageFormat{storageFormat} {}
     BoundExtraCreateNodeTableInfo(const BoundExtraCreateNodeTableInfo& other)
         : BoundExtraCreateTableInfo{copyVector(other.propertyDefinitions)},
           primaryKeyName{other.primaryKeyName}, storage{other.storage},
@@ -100,22 +101,22 @@ struct BoundExtraCreateRelTableGroupInfo final : BoundExtraCreateTableInfo {
     common::ExtendDirection storageDirection;
     std::vector<catalog::NodeTableIDPair> nodePairs;
     std::string storage;
-    std::string storageFormat;
+    common::StorageFormat storageFormat = common::StorageFormat::NONE;
     std::optional<function::TableFunction> scanFunction;
     std::optional<std::shared_ptr<function::TableFuncBindData>> scanBindData;
     std::string foreignDatabaseName;
 
     explicit BoundExtraCreateRelTableGroupInfo(std::vector<PropertyDefinition> definitions,
         common::RelMultiplicity srcMultiplicity, common::RelMultiplicity dstMultiplicity,
         common::ExtendDirection storageDirection, std::vector<catalog::NodeTableIDPair> nodePairs,
-        std::string storage = "", std::string storageFormat = "",
+        std::string storage = "", common::StorageFormat storageFormat = common::StorageFormat::NONE,
         std::optional<function::TableFunction> scanFunction = std::nullopt,
         std::optional<std::shared_ptr<function::TableFuncBindData>> scanBindData = std::nullopt,
         std::string foreignDatabaseName = "")
         : BoundExtraCreateTableInfo{std::move(definitions)}, srcMultiplicity{srcMultiplicity},
           dstMultiplicity{dstMultiplicity}, storageDirection{storageDirection},
           nodePairs{std::move(nodePairs)}, storage{std::move(storage)},
-          storageFormat{std::move(storageFormat)}, scanFunction{std::move(scanFunction)},
+          storageFormat{storageFormat}, scanFunction{std::move(scanFunction)},
           scanBindData{std::move(scanBindData)},
           foreignDatabaseName{std::move(foreignDatabaseName)} {}
 

src/include/catalog/catalog_entry/node_table_catalog_entry.h

@@ -3,6 +3,7 @@
 #include <functional>
 #include <optional>
 
+#include "common/enums/storage_format.h"
 #include "function/table/table_function.h"
 #include "table_catalog_entry.h"
 
@@ -28,9 +29,9 @@ class LBUG_API NodeTableCatalogEntry final : public TableCatalogEntry {
 public:
     NodeTableCatalogEntry() = default;
     NodeTableCatalogEntry(std::string name, std::string primaryKeyName, std::string storage = "",
-        std::string storageFormat = "")
+        common::StorageFormat storageFormat = common::StorageFormat::NONE)
         : TableCatalogEntry{entryType_, std::move(name)}, primaryKeyName{std::move(primaryKeyName)},
-          storage{std::move(storage)}, storageFormat{std::move(storageFormat)} {}
+          storage{std::move(storage)}, storageFormat{storageFormat} {}
 
     // Constructor for foreign-backed tables
     NodeTableCatalogEntry(std::string name, std::string primaryKeyName,
@@ -57,7 +58,7 @@ class LBUG_API NodeTableCatalogEntry final : public TableCatalogEntry {
         return getProperty(primaryKeyName);
     }
     const std::string& getStorage() const { return storage; }
-    const std::string& getStorageFormat() const { return storageFormat; }
+    common::StorageFormat getStorageFormat() const { return storageFormat; }
     std::optional<function::TableFunction> getScanFunction() const override;
     const CreateBindDataFunc& getCreateBindDataFunc() const { return createBindDataFunc; }
     const std::string& getForeignDatabaseName() const { return foreignDatabaseName; }
@@ -84,7 +85,7 @@ class LBUG_API NodeTableCatalogEntry final : public TableCatalogEntry {
 private:
     std::string primaryKeyName;
     std::string storage;
-    std::string storageFormat;
+    common::StorageFormat storageFormat = common::StorageFormat::NONE;
     std::optional<function::TableFunction> scanFunction;
     CreateBindDataFunc createBindDataFunc; // Callback to create bind data
     std::string foreignDatabaseName;