From 8f903521fdf9c8e9f94720db7a3bc01ebaf287e6 Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Sun, 10 May 2026 20:26:14 +0530 Subject: [PATCH 01/10] refactor parquet -> icebug-disk --- docs/icebug-disk.md | 50 +++++++ docs/morsel_parallelism.md | 2 +- src/binder/bind/bind_ddl.cpp | 25 ++++ .../catalog_entry/rel_group_catalog_entry.cpp | 2 +- src/graph/on_disk_graph.cpp | 6 +- src/include/common/constants.h | 6 + .../reader/parquet/parquet_reader.h | 5 + src/include/storage/storage_manager.h | 7 + .../storage/table/columnar_node_table_base.h | 8 +- .../storage/table/columnar_rel_table_base.h | 15 +- .../storage/table/ice_disk_constants.h | 18 +++ ...uet_node_table.h => ice_disk_node_table.h} | 14 +- ...rquet_rel_table.h => ice_disk_rel_table.h} | 15 +- src/include/storage/table/ice_disk_utils.h | 133 +++++++++++++++++ src/include/storage/table/rel_table.h | 2 +- .../reader/parquet/parquet_reader.cpp | 37 +++++ .../operator/scan/scan_multi_rel_tables.cpp | 18 +-- .../operator/scan/scan_node_table.cpp | 20 +-- .../operator/scan/scan_rel_table.cpp | 8 +- src/storage/storage_manager.cpp | 24 ++-- src/storage/table/CMakeLists.txt | 4 +- src/storage/table/arrow_rel_table.cpp | 2 +- ...node_table.cpp => ice_disk_node_table.cpp} | 136 +++++++++--------- ...t_rel_table.cpp => ice_disk_rel_table.cpp} | 119 +++++++-------- test/include/test_runner/test_group.h | 2 +- test/runner/e2e_test.cpp | 4 +- ...raph_std.test => demo_db_icebug_disk.test} | 4 +- test/test_helper/test_helper.cpp | 40 +++++- test/test_runner/test_parser.cpp | 4 +- 29 files changed, 516 insertions(+), 214 deletions(-) create mode 100644 docs/icebug-disk.md create mode 100644 src/include/storage/table/ice_disk_constants.h rename src/include/storage/table/{parquet_node_table.h => ice_disk_node_table.h} (89%) rename src/include/storage/table/{parquet_rel_table.h => ice_disk_rel_table.h} (87%) create mode 100644 src/include/storage/table/ice_disk_utils.h rename src/storage/table/{parquet_node_table.cpp => ice_disk_node_table.cpp} (73%) rename src/storage/table/{parquet_rel_table.cpp => ice_disk_rel_table.cpp} (75%) rename test/test_files/demo_db/{demo_db_graph_std.test => demo_db_icebug_disk.test} (98%) diff --git a/docs/icebug-disk.md b/docs/icebug-disk.md new file mode 100644 index 0000000000..98a925fcc3 --- /dev/null +++ b/docs/icebug-disk.md @@ -0,0 +1,50 @@ +# Icebug-Disk Storage Format + +## Overview + +This is LadybugDB's implementation of [Icebug-Disk](https://github.com/Ladybug-Memory/icebug-format), a read-only graph storage format based on Parquet files. It is designed for efficient analytical queries on large graphs. + +## V1 + +Implements Icebug-Disk v1. + +### Version + +Version is stored in each file's metadata footer as a key-value pair: `icebug_disk_version = v1`. + +### schema.cypher + +The graph schema is declared in a `schema.cypher`, which can be loaded using `lbug -i schema.cypher` to create tables in Ladybug. + +```cypher +CREATE NODE TABLE city(id INT32, name STRING, population INT64, PRIMARY KEY(id)) WITH (storage = 'icebug-disk:'); +CREATE NODE TABLE user(id INT32, name STRING, age INT64, PRIMARY KEY(id)) WITH (storage = 'icebug-disk:'); +CREATE REL TABLE follows(FROM user TO user, since INT32) WITH (storage = 'icebug-disk:'); +CREATE REL TABLE livesin(FROM user TO city) WITH (storage = 'icebug-disk:'); +``` + +File paths can be relative or absolute and are resolved as `/nodes_{tableName}.parquet` for node tables, and `/indices_{tableName}.parquet` and `/indptr_{tableName}.parquet` for relationship tables. + +`storage = 'icebug-disk'` and `storage = 'icebug-disk:'` both resolve to the current working directory. + +Tables can also be created by manually running the above queries in the Ladybug CLI. + +If the directory is moved, the affected tables must be dropped and re-created with the updated path. This updates the file pointers in the catalog. A fresh db instance can also be used to run the `CREATE TABLE` queries with the new paths. + +Mixed tables are not supported — `CREATE REL TABLE` queries involving both `icebug-disk` and non-`icebug-disk` tables will throw a `BinderException`. + +### Node tables + +For each node table, there is a corresponding Parquet file named `nodes_{tableName}.parquet` containing a primary key column and one column per property as declared in the schema. + +### Indices + +Each relationship table has a corresponding `indices_{tableName}.parquet` file containing one row per edge. The first column is always `target` (the destination node offset), followed by zero or more edge property columns as declared in the schema. + +### Indptr + +Each relationship table has a corresponding `indptr_{tableName}.parquet` file containing the CSR row pointers. It has a single integer column with `N+1` entries, where `N` is the number of source nodes. + +## Convert from other formats + +You can convert from other graph formats (e.g. duckdb, parquet tables) to Icebug-Disk using the script at https://github.com/Ladybug-Memory/icebug-format diff --git a/docs/morsel_parallelism.md b/docs/morsel_parallelism.md index ee7f20f935..6108ee0602 100644 --- a/docs/morsel_parallelism.md +++ b/docs/morsel_parallelism.md @@ -2,7 +2,7 @@ ## Overview -Morsel-driven parallelism is ladybug's approach to parallel query execution where work is divided into small, independently processable chunks called "morsels." This document explains how it works for native node tables versus columnar formats (Arrow, Parquet). +Morsel-driven parallelism is ladybug's approach to parallel query execution where work is divided into small, independently processable chunks called "morsels." This document explains how it works for native node tables versus columnar formats (Arrow, Icebug Disk). ## Architecture diff --git a/src/binder/bind/bind_ddl.cpp b/src/binder/bind/bind_ddl.cpp index 9c4c702791..4026f09fd8 100644 --- a/src/binder/bind/bind_ddl.cpp +++ b/src/binder/bind/bind_ddl.cpp @@ -11,6 +11,7 @@ #include "catalog/catalog.h" #include "catalog/catalog_entry/node_table_catalog_entry.h" #include "catalog/catalog_entry/sequence_catalog_entry.h" +#include "common/constants.h" #include "common/enums/extend_direction_util.h" #include "common/exception/binder.h" #include "common/exception/message.h" @@ -319,6 +320,30 @@ BoundCreateTableInfo Binder::bindCreateRelTableGroupInfo(const CreateTableInfo* } } + // We only allow icebug-disk rel tables to connect icebug-disk node tables + if (TableOptionConstants::isIceBugDiskStorage(storage)) { + auto srcNodeEntry = srcEntry->ptrCast(); + auto dstNodeEntry = dstEntry->ptrCast(); + + if (!TableOptionConstants::isIceBugDiskStorage(srcNodeEntry->getStorage()) || + !TableOptionConstants::isIceBugDiskStorage(dstNodeEntry->getStorage())) { + throw BinderException("icebug-disk rel tables require both FROM and TO tables to " + "be icebug-disk node tables."); + } + } + + // Non-icebug-disk rel tables cannot connect to icebug-disk node tables + if (!TableOptionConstants::isIceBugDiskStorage(storage)) { + auto srcNodeEntry = srcEntry->ptrCast(); + auto dstNodeEntry = dstEntry->ptrCast(); + + if (TableOptionConstants::isIceBugDiskStorage(srcNodeEntry->getStorage()) || + TableOptionConstants::isIceBugDiskStorage(dstNodeEntry->getStorage())) { + throw BinderException("Rel tables with non-icebug-disk storage do not support " + "icebug-disk node tables as FROM or TO tables."); + } + } + // Use the actual shadow table IDs, not FOREIGN_TABLE_ID // The shadow tables allow the query planner to distinguish between different node tables auto srcTableID = srcEntry->getTableID(); diff --git a/src/catalog/catalog_entry/rel_group_catalog_entry.cpp b/src/catalog/catalog_entry/rel_group_catalog_entry.cpp index 46da2a4fbe..4b9f145bf2 100644 --- a/src/catalog/catalog_entry/rel_group_catalog_entry.cpp +++ b/src/catalog/catalog_entry/rel_group_catalog_entry.cpp @@ -214,7 +214,7 @@ RelGroupCatalogEntry::getBoundExtraCreateInfo(transaction::Transaction*) const { } return std::make_unique( copyVector(propertyCollection.getDefinitions()), srcMultiplicity, dstMultiplicity, - storageDirection, std::move(nodePairs)); + storageDirection, std::move(nodePairs), storage); } } // namespace catalog diff --git a/src/graph/on_disk_graph.cpp b/src/graph/on_disk_graph.cpp index 6f4dd077f4..5ebf9acc21 100644 --- a/src/graph/on_disk_graph.cpp +++ b/src/graph/on_disk_graph.cpp @@ -18,8 +18,8 @@ #include "storage/local_storage/local_storage.h" #include "storage/storage_manager.h" #include "storage/storage_utils.h" +#include "storage/table/ice_disk_rel_table.h" #include "storage/table/node_table.h" -#include "storage/table/parquet_rel_table.h" #include "storage/table/rel_table.h" using namespace lbug::catalog; @@ -129,8 +129,8 @@ OnDiskGraphNbrScanState::OnDiskGraphNbrScanState(ClientContext* context, } std::unique_ptr scanState; - if (dynamic_cast(table) != nullptr) { - scanState = std::make_unique(*mm, srcNodeIDVector.get(), + if (dynamic_cast(table) != nullptr) { + scanState = std::make_unique(*mm, srcNodeIDVector.get(), outVectors, state); } else { scanState = std::make_unique(*MemoryManager::Get(*context), diff --git a/src/include/common/constants.h b/src/include/common/constants.h index dcbbb43b53..536ee2176c 100644 --- a/src/include/common/constants.h +++ b/src/include/common/constants.h @@ -86,6 +86,12 @@ struct StorageConstants { struct TableOptionConstants { static constexpr char REL_STORAGE_DIRECTION_OPTION[] = "STORAGE_DIRECTION"; static constexpr char REL_STORAGE_OPTION[] = "STORAGE"; + + static constexpr std::string_view ICEBUG_DISK_PREFIX = "icebug-disk"; + + static bool isIceBugDiskStorage(const std::string& storage) { + return storage.starts_with(ICEBUG_DISK_PREFIX); + } }; // Hash Index Configurations diff --git a/src/include/processor/operator/persistent/reader/parquet/parquet_reader.h b/src/include/processor/operator/persistent/reader/parquet/parquet_reader.h index c0a2cedd5f..ea89c1d516 100644 --- a/src/include/processor/operator/persistent/reader/parquet/parquet_reader.h +++ b/src/include/processor/operator/persistent/reader/parquet/parquet_reader.h @@ -56,6 +56,11 @@ class ParquetReader { lbug_parquet::format::FileMetaData* getMetadata() const { return metadata.get(); } + // Read only the parquet footer metadata from a file without requiring a ClientContext. + // Used for version checks at table construction time when no context is available. + static std::unique_ptr readMetadata( + const std::string& filePath, common::VirtualFileSystem* vfs); + private: std::unique_ptr createThriftProtocol( common::FileInfo* fileInfo_, bool prefetch_mode) { diff --git a/src/include/storage/storage_manager.h b/src/include/storage/storage_manager.h index 3268e0ad3e..409708ccdd 100644 --- a/src/include/storage/storage_manager.h +++ b/src/include/storage/storage_manager.h @@ -12,6 +12,10 @@ namespace main { class Database; } // namespace main +namespace common { +class VirtualFileSystem; +} // namespace common + namespace catalog { class CatalogEntry; class Catalog; @@ -65,6 +69,8 @@ class LBUG_API StorageManager { bool defaultHashIndexEnabled() const { return enableDefaultHashIndex; } void setDefaultHashIndexEnabled(bool enabled) { enableDefaultHashIndex = enabled; } + common::VirtualFileSystem* getVFS() const { return vfs_; } + void registerIndexType(IndexType indexType) { registeredIndexTypes.push_back(std::move(indexType)); } @@ -115,6 +121,7 @@ class LBUG_API StorageManager { bool inMemory; std::vector registeredIndexTypes; std::unordered_map tableNameCache; + common::VirtualFileSystem* vfs_; // non-owning, owned by Database }; } // namespace storage diff --git a/src/include/storage/table/columnar_node_table_base.h b/src/include/storage/table/columnar_node_table_base.h index fed8e34492..a719aa8402 100644 --- a/src/include/storage/table/columnar_node_table_base.h +++ b/src/include/storage/table/columnar_node_table_base.h @@ -32,7 +32,7 @@ struct ColumnarNodeTableScanSharedState { virtual bool getNextMorsel(ColumnarNodeTableScanState* scanState) = 0; }; -// Abstract base class for columnar-format node tables (Parquet, Arrow, etc.) +// Abstract base class for columnar-format node tables (Icebug-Disk, Arrow, etc.) class ColumnarNodeTableBase : public NodeTable { public: ColumnarNodeTableBase(const StorageManager* storageManager, @@ -77,12 +77,6 @@ class ColumnarNodeTableBase : public NodeTable { virtual common::row_idx_t getTotalRowCount( const transaction::Transaction* transaction) const = 0; - // Helper for constructing storage paths - std::string constructStoragePath(const std::string& prefix, const std::string& suffix) const { - std::string tableName = nodeTableCatalogEntry->getName(); - return prefix + "_nodes_" + tableName + suffix; - } - public: ColumnarNodeTableScanSharedState* getTableScanSharedState() const { return tableScanSharedState.get(); diff --git a/src/include/storage/table/columnar_rel_table_base.h b/src/include/storage/table/columnar_rel_table_base.h index 5f04510616..fad0595d83 100644 --- a/src/include/storage/table/columnar_rel_table_base.h +++ b/src/include/storage/table/columnar_rel_table_base.h @@ -12,7 +12,7 @@ namespace lbug { namespace storage { -// Abstract base class for columnar-format relationship tables (Parquet, Arrow, etc.) +// Abstract base class for columnar-format relationship tables (Icebug-Disk, Arrow, etc.) class ColumnarRelTableBase : public RelTable { public: ColumnarRelTableBase(catalog::RelGroupCatalogEntry* relGroupEntry, @@ -54,19 +54,6 @@ class ColumnarRelTableBase : public RelTable { virtual common::row_idx_t getTotalRowCount( const transaction::Transaction* transaction) const = 0; - // Helper for constructing storage paths for CSR format - struct CSRFilePaths { - std::string indices; - std::string indptr; - std::string metadata; - }; - - CSRFilePaths constructCSRPaths(const std::string& prefix, const std::string& suffix) const { - std::string relName = relGroupEntry->getName(); - return {prefix + "_indices_" + relName + suffix, prefix + "_indptr_" + relName + suffix, - prefix + "_metadata_" + relName + suffix}; - } - // Helper for finding source node in CSR format // Subclasses should cache indptr data and provide it via this interface virtual common::offset_t findSourceNodeForRowInternal(common::offset_t globalRowIdx, diff --git a/src/include/storage/table/ice_disk_constants.h b/src/include/storage/table/ice_disk_constants.h new file mode 100644 index 0000000000..7e6a9b5d3c --- /dev/null +++ b/src/include/storage/table/ice_disk_constants.h @@ -0,0 +1,18 @@ +#pragma once + +#include + +#include "common/constants.h" + +namespace lbug { +namespace storage { + +struct IceDiskConstants { + static constexpr std::string_view V1 = "v1"; + + static constexpr std::string_view CURRENT_VERSION = V1; + + static constexpr std::string_view VERSION_METADATA_KEY = "icebug_disk_version"; +}; +} // namespace storage +} // namespace lbug diff --git a/src/include/storage/table/parquet_node_table.h b/src/include/storage/table/ice_disk_node_table.h similarity index 89% rename from src/include/storage/table/parquet_node_table.h rename to src/include/storage/table/ice_disk_node_table.h index dcf681931e..eb64cfc478 100644 --- a/src/include/storage/table/parquet_node_table.h +++ b/src/include/storage/table/ice_disk_node_table.h @@ -14,7 +14,7 @@ namespace lbug { namespace storage { -struct ParquetNodeTableScanState final : ColumnarNodeTableScanState { +struct IceDiskNodeTableScanState final : ColumnarNodeTableScanState { std::unique_ptr parquetReader; std::unique_ptr parquetScanState; bool dataRead = false; @@ -22,7 +22,7 @@ struct ParquetNodeTableScanState final : ColumnarNodeTableScanState { size_t nextRowToDistribute = 0; uint64_t lastQueryId = 0; // Track the last query ID to detect new queries - ParquetNodeTableScanState([[maybe_unused]] MemoryManager& mm, common::ValueVector* nodeIDVector, + IceDiskNodeTableScanState([[maybe_unused]] MemoryManager& mm, common::ValueVector* nodeIDVector, std::vector outputVectors, std::shared_ptr outChunkState) : ColumnarNodeTableScanState{mm, nodeIDVector, std::move(outputVectors), @@ -31,7 +31,7 @@ struct ParquetNodeTableScanState final : ColumnarNodeTableScanState { } }; -struct ParquetNodeTableScanSharedState final : ColumnarNodeTableScanSharedState { +struct IceDiskNodeTableScanSharedState final : ColumnarNodeTableScanSharedState { private: std::mutex mtx; common::node_group_idx_t currentBatchIdx = 0; @@ -58,9 +58,9 @@ struct ParquetNodeTableScanSharedState final : ColumnarNodeTableScanSharedState bool getNextMorsel(ColumnarNodeTableScanState*) override { return false; } }; -class ParquetNodeTable final : public ColumnarNodeTableBase { +class IceDiskNodeTable final : public ColumnarNodeTableBase { public: - ParquetNodeTable(const StorageManager* storageManager, + IceDiskNodeTable(const StorageManager* storageManager, const catalog::NodeTableCatalogEntry* nodeTableEntry, MemoryManager* memoryManager); void initializeScanCoordination(const transaction::Transaction* transaction) override; @@ -79,7 +79,7 @@ class ParquetNodeTable final : public ColumnarNodeTableBase { protected: // Implement ColumnarNodeTableBase interface - std::string getColumnarFormatName() const override { return "Parquet"; } + std::string getColumnarFormatName() const override { return "icebug-"; } common::node_group_idx_t getNumBatches( const transaction::Transaction* transaction) const override; common::row_idx_t getTotalRowCount(const transaction::Transaction* transaction) const override; @@ -90,7 +90,7 @@ class ParquetNodeTable final : public ColumnarNodeTableBase { void initializeParquetReader(transaction::Transaction* transaction) const; void initParquetScanForRowGroup(transaction::Transaction* transaction, - ParquetNodeTableScanState& scanState) const; + IceDiskNodeTableScanState& iceDiskScanState) const; }; } // namespace storage diff --git a/src/include/storage/table/parquet_rel_table.h b/src/include/storage/table/ice_disk_rel_table.h similarity index 87% rename from src/include/storage/table/parquet_rel_table.h rename to src/include/storage/table/ice_disk_rel_table.h index c63a373a7f..ec7488c42a 100644 --- a/src/include/storage/table/parquet_rel_table.h +++ b/src/include/storage/table/ice_disk_rel_table.h @@ -10,7 +10,7 @@ namespace lbug { namespace storage { -struct ParquetRelTableScanState final : RelTableScanState { +struct IceDiskRelTableScanState final : RelTableScanState { std::unique_ptr parquetScanState; // Row group range for morsel-driven parallelism @@ -21,7 +21,7 @@ struct ParquetRelTableScanState final : RelTableScanState { std::unique_ptr indicesReader; std::unique_ptr indptrReader; - ParquetRelTableScanState(MemoryManager& mm, common::ValueVector* nodeIDVector, + IceDiskRelTableScanState(MemoryManager& mm, common::ValueVector* nodeIDVector, std::vector outputVectors, std::shared_ptr outChunkState) : RelTableScanState{mm, nodeIDVector, std::move(outputVectors), std::move(outChunkState)} { @@ -34,9 +34,9 @@ struct ParquetRelTableScanState final : RelTableScanState { common::RelDataDirection direction_) override; }; -class ParquetRelTable final : public ColumnarRelTableBase { +class IceDiskRelTable final : public ColumnarRelTableBase { public: - ParquetRelTable(catalog::RelGroupCatalogEntry* relGroupEntry, common::table_id_t fromTableID, + IceDiskRelTable(catalog::RelGroupCatalogEntry* relGroupEntry, common::table_id_t fromTableID, common::table_id_t toTableID, const StorageManager* storageManager, MemoryManager* memoryManager); @@ -47,7 +47,7 @@ class ParquetRelTable final : public ColumnarRelTableBase { protected: // Implement ColumnarRelTableBase interface - std::string getColumnarFormatName() const override { return "Parquet"; } + std::string getColumnarFormatName() const override { return "icebug-disk"; } common::row_idx_t getTotalRowCount(const transaction::Transaction* transaction) const override; private: @@ -63,10 +63,9 @@ class ParquetRelTable final : public ColumnarRelTableBase { void initializeIndptrReader(transaction::Transaction* transaction) const; void loadIndptrData(transaction::Transaction* transaction) const; bool scanInternalByRowGroups(transaction::Transaction* transaction, - ParquetRelTableScanState& parquetRelScanState); + IceDiskRelTableScanState& iceDiskScanState); bool scanRowGroupForBoundNodes(transaction::Transaction* transaction, - ParquetRelTableScanState& parquetRelScanState, - const std::vector& rowGroupsToProcess, + IceDiskRelTableScanState& iceDiskScanState, const std::vector& rowGroupsToProcess, const std::unordered_map& boundNodeOffsets); common::offset_t findSourceNodeForRow(common::offset_t globalRowIdx) const; }; diff --git a/src/include/storage/table/ice_disk_utils.h b/src/include/storage/table/ice_disk_utils.h new file mode 100644 index 0000000000..cc976ec002 --- /dev/null +++ b/src/include/storage/table/ice_disk_utils.h @@ -0,0 +1,133 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + +#include "common/constants.h" +#include "common/exception/runtime.h" +#include "common/file_system/virtual_file_system.h" +#include "processor/operator/persistent/reader/parquet/parquet_reader.h" +#include "storage/table/ice_disk_constants.h" + +namespace lbug { +namespace storage { + +struct CSRFilePaths { + std::string indices; + std::string indptr; +}; + +class IceDiskUtils { +public: + // Parses "icebug-disk", "icebug-disk:", or "icebug-disk:" and returns the path + // component. Returns empty string for the first two forms (caller interprets as current dir) + static std::string getBasePath(const std::string& storage) { + if (!storage.starts_with(common::TableOptionConstants::ICEBUG_DISK_PREFIX)) { + return ""; + } + std::string_view rest = std::string_view(storage).substr( + common::TableOptionConstants::ICEBUG_DISK_PREFIX.size()); + // Strip the optional ':' separator. + if (!rest.empty() && rest[0] == ':') { + rest = rest.substr(1); + } + return std::string(rest); // empty means "current directory" + } + + // Joins a base path with a filename. When base is empty the filename is returned + // as-is (i.e. relative to the current working directory) + static std::string joinPath(const std::string& base, const std::string& part) { + if (base.empty()) { + return part; + } + const char last = base.back(); + if (last == '/' || last == '\\') { + return base + part; + } + + return base + "/" + part; + } + + // Get the file path for a given node table's parquet file + static std::string constructNodeTablePath(const std::string& dir, const std::string& name, + const std::string& suffix) { + return IceDiskUtils::joinPath(dir, "nodes_" + name + suffix); + } + + // Get the file paths for a given rel table's CSR files + static CSRFilePaths constructCSRPaths(const std::string& dir, const std::string& name, + const std::string& suffix) { + return {IceDiskUtils::joinPath(dir, "indices_" + name + suffix), + IceDiskUtils::joinPath(dir, "indptr_" + name + suffix)}; + } + + // Resolves a potentially relative or ~-prefixed path against dbDirectory. + static std::string resolveIceDiskPath(const std::string& path, const std::string& dbDirectory) { + std::string expanded = path; + if (!expanded.empty() && expanded[0] == '~') { + const char* home = std::getenv("HOME"); + if (!home) { + throw common::RuntimeException( + "Cannot expand '~' in IceDisk path without HOME set: " + path); + } + expanded = std::string(home) + expanded.substr(1); + } + if (!std::filesystem::path(expanded).is_absolute()) { + expanded = (std::filesystem::path(dbDirectory) / expanded).lexically_normal().string(); + } + return expanded; + } + + // Validates that the parquet file at `path` carries the expected icebug_disk_version metadata. + // `dbDirectory` is used to anchor relative paths (typically parent of the .ladybug file). + static void checkVersionCompatibility(common::VirtualFileSystem* vfs, + const std::string& dbDirectory, const std::string& path) { + if (!vfs) { + throw common::RuntimeException( + "No VirtualFileSystem available for IceDisk version check: " + path); + } + + const auto resolvedPath = resolveIceDiskPath(path, dbDirectory); + + auto metadata = processor::ParquetReader::readMetadata(resolvedPath, vfs); + if (!metadata) { + throw common::RuntimeException( + path + ": failed to read parquet metadata for version check"); + } + + // key_value_metadata is std::vector + bool found = false; + std::string versionValue; + for (const auto& kv : metadata->key_value_metadata) { + if (kv.key == IceDiskConstants::VERSION_METADATA_KEY) { + versionValue = kv.value; + found = true; + break; + } + } + + if (!found) { + throw common::RuntimeException( + path + ": parquet file is missing icebug_disk_version metadata"); + } + + const auto& expected = IceDiskConstants::CURRENT_VERSION; + const bool versionMatches = versionValue.size() == expected.size() && + std::equal(versionValue.begin(), versionValue.end(), + expected.begin(), [](unsigned char a, unsigned char b) { + return std::tolower(a) == std::tolower(b); + }); + + if (!versionMatches) { + throw common::RuntimeException( + "Current ladybug version does not support icebug_disk_version: " + versionValue); + } + } +}; + +} // namespace storage +} // namespace lbug diff --git a/src/include/storage/table/rel_table.h b/src/include/storage/table/rel_table.h index 696985463a..5bee7d6223 100644 --- a/src/include/storage/table/rel_table.h +++ b/src/include/storage/table/rel_table.h @@ -30,7 +30,7 @@ struct RelTableScanState : TableScanState { std::unique_ptr localTableScanState; // Optional state used by Arrow-backed relationship tables. Keep it on the common scan state so - // a single multi-rel scan state can scan native, Parquet-backed, and Arrow-backed tables. + // a single multi-rel scan state can scan native, icebug-disk-backed, and Arrow-backed tables. size_t arrowCurrentBatchIdx = 0; size_t arrowCurrentBatchOffset = 0; std::unordered_map arrowBoundNodeOffsetToSelPos; diff --git a/src/processor/operator/persistent/reader/parquet/parquet_reader.cpp b/src/processor/operator/persistent/reader/parquet/parquet_reader.cpp index 0972426319..2879a37d36 100644 --- a/src/processor/operator/persistent/reader/parquet/parquet_reader.cpp +++ b/src/processor/operator/persistent/reader/parquet/parquet_reader.cpp @@ -219,6 +219,43 @@ void ParquetReader::initMetadata() { metadata->read(proto.get()); } +std::unique_ptr ParquetReader::readMetadata(const std::string& filePath, + VirtualFileSystem* vfs) { + auto fileInfo = vfs->openFile(filePath, FileOpenFlags(FileFlags::READ_ONLY), nullptr); + auto proto = + std::make_unique>( + std::make_shared(fileInfo.get(), false)); + auto& transport = dynamic_cast_checked(*proto->getTransport()); + auto fileSize = transport.GetSize(); + if (fileSize < 12) { + throw CopyException{ + std::format("File {} is too small to be a Parquet file", filePath.c_str())}; + } + + ResizeableBuffer buf; + buf.resize(8); + buf.zero(); + + transport.SetLocation(fileSize - 8); + transport.read((uint8_t*)buf.ptr, 8); + + if (memcmp(buf.ptr + 4, "PAR1", 4) != 0) { + throw CopyException{ + std::format("No magic bytes found at the end of file {}", filePath.c_str())}; + } + auto footerLen = *reinterpret_cast(buf.ptr); + if (footerLen == 0 || fileSize < 12 + footerLen) { + throw CopyException{std::format("Footer length error in file {}", filePath.c_str())}; + } + auto metadataPos = fileSize - (footerLen + 8); + transport.SetLocation(metadataPos); + transport.Prefetch(metadataPos, footerLen); + + auto result = std::make_unique(); + result->read(proto.get()); + return result; +} + std::unique_ptr ParquetReader::createReaderRecursive(uint64_t depth, uint64_t maxDefine, uint64_t maxRepeat, uint64_t& nextSchemaIdx, uint64_t& nextFileIdx) { DASSERT(nextSchemaIdx < metadata->schema.size()); diff --git a/src/processor/operator/scan/scan_multi_rel_tables.cpp b/src/processor/operator/scan/scan_multi_rel_tables.cpp index 27f5422379..f24eae254b 100644 --- a/src/processor/operator/scan/scan_multi_rel_tables.cpp +++ b/src/processor/operator/scan/scan_multi_rel_tables.cpp @@ -3,7 +3,7 @@ #include "processor/execution_context.h" #include "storage/local_storage/local_storage.h" #include "storage/table/arrow_rel_table.h" -#include "storage/table/parquet_rel_table.h" +#include "storage/table/ice_disk_rel_table.h" using namespace lbug::common; using namespace lbug::storage; @@ -68,28 +68,28 @@ void ScanMultiRelTable::initLocalStateInternal(ResultSet* resultSet, ExecutionCo // Check if any table in any scanner is an external rel table with a custom scan state. bool hasArrowTable = false; - bool hasParquetTable = false; + bool hasIceDiskTable = false; for (auto& [_, scanner] : scanners) { for (auto& relInfo : scanner.relInfos) { if (dynamic_cast(relInfo.table) != nullptr) { hasArrowTable = true; break; } - if (dynamic_cast(relInfo.table) != nullptr) { - hasParquetTable = true; + if (dynamic_cast(relInfo.table) != nullptr) { + hasIceDiskTable = true; break; } } - if (hasArrowTable || hasParquetTable) { + if (hasArrowTable || hasIceDiskTable) { break; } } - // Parquet scan state extends the common rel scan state and Arrow stores its per-table state - // there, so one scan state can now cover Parquet, Arrow, and native rel tables. - if (hasParquetTable) { + // IceDisk scan state extends the common rel scan state and Arrow stores its per-table state + // there, so one scan state can now cover IceDisk, Arrow, and native rel tables. + if (hasIceDiskTable) { scanState = - std::make_unique(*MemoryManager::Get(*clientContext), + std::make_unique(*MemoryManager::Get(*clientContext), boundNodeIDVector, outVectors, nbrNodeIDVector->state); } else if (hasArrowTable) { scanState = diff --git a/src/processor/operator/scan/scan_node_table.cpp b/src/processor/operator/scan/scan_node_table.cpp index 9496756cf0..e9c9524854 100644 --- a/src/processor/operator/scan/scan_node_table.cpp +++ b/src/processor/operator/scan/scan_node_table.cpp @@ -7,7 +7,7 @@ #include "storage/local_storage/local_node_table.h" #include "storage/local_storage/local_storage.h" #include "storage/table/arrow_node_table.h" -#include "storage/table/parquet_node_table.h" +#include "storage/table/ice_disk_node_table.h" using namespace lbug::common; using namespace lbug::storage; @@ -17,8 +17,8 @@ namespace processor { static std::unique_ptr createNodeTableScanState(NodeTable* table, ValueVector* nodeIDVector, const std::vector& outVectors, MemoryManager* memoryManager) { - if (dynamic_cast(table) != nullptr) { - return std::make_unique(*memoryManager, nodeIDVector, outVectors, + if (dynamic_cast(table) != nullptr) { + return std::make_unique(*memoryManager, nodeIDVector, outVectors, nodeIDVector->state); } if (dynamic_cast(table) != nullptr) { @@ -53,16 +53,16 @@ void ScanNodeTableSharedState::initialize(const transaction::Transaction* transa this->currentCommittedGroupIdx = 0; this->currentUnCommittedGroupIdx = 0; - // Initialize table-specific scan coordination (e.g., for ParquetNodeTable) + // Initialize table-specific scan coordination (e.g., for IceDiskNodeTable) table->initializeScanCoordination(transaction); - if (const auto parquetTable = dynamic_cast(table)) { - // For parquet tables, set numCommittedNodeGroups to number of row groups + if (const auto iceDiskTable = dynamic_cast(table)) { + // For ice-disk tables, set numCommittedNodeGroups to number of row groups std::vector columnSkips; try { auto context = transaction->getClientContext(); auto resolvedPath = - common::VirtualFileSystem::resolvePath(context, parquetTable->getParquetFilePath()); + common::VirtualFileSystem::resolvePath(context, iceDiskTable->getParquetFilePath()); auto tempReader = std::make_unique(resolvedPath, columnSkips, context); this->numCommittedNodeGroups = tempReader->getNumRowGroups(); @@ -91,7 +91,7 @@ void ScanNodeTableSharedState::nextMorsel(TableScanState& scanState, std::unique_lock lck{mtx}; // ColumnarNodeTables handle morsel assignment internally - // TODO: parquet tables https://github.com/LadybugDB/ladybug/issues/245 + // TODO: icebug-disk tables https://github.com/LadybugDB/ladybug/issues/245 if (const auto arrowTable = dynamic_cast(this->table)) { const auto tableSharedState = arrowTable->getTableScanSharedState(); if (tableSharedState->getNextMorsel(static_cast(&scanState))) { @@ -149,8 +149,8 @@ void ScanNodeTable::initCurrentTable(ExecutionContext* context) { outVectors, MemoryManager::Get(*context->clientContext)); currentInfo.initScanState(*scanState, outVectors, context->clientContext); scanState->semiMask = sharedStates[currentTableIdx]->getSemiMask(); - // Call table->initScanState for ParquetNodeTable or ArrowNodeTable - if (dynamic_cast(tableInfos[currentTableIdx].table) || + // Call table->initScanState for IceDiskNodeTable or ArrowNodeTable + if (dynamic_cast(tableInfos[currentTableIdx].table) || dynamic_cast(tableInfos[currentTableIdx].table)) { auto transaction = transaction::Transaction::Get(*context->clientContext); tableInfos[currentTableIdx].table->initScanState(transaction, *scanState); diff --git a/src/processor/operator/scan/scan_rel_table.cpp b/src/processor/operator/scan/scan_rel_table.cpp index f09b7a9abd..de7c4051df 100644 --- a/src/processor/operator/scan/scan_rel_table.cpp +++ b/src/processor/operator/scan/scan_rel_table.cpp @@ -9,8 +9,8 @@ #include "storage/local_storage/local_rel_table.h" #include "storage/table/arrow_rel_table.h" #include "storage/table/foreign_rel_table.h" +#include "storage/table/ice_disk_rel_table.h" #include "storage/table/node_table.h" -#include "storage/table/parquet_rel_table.h" using namespace lbug::common; using namespace lbug::storage; @@ -76,15 +76,15 @@ void ScanRelTable::initLocalStateInternal(ResultSet* resultSet, ExecutionContext auto nbrNodeIDVector = outVectors[0]; // Check if this is an external rel table and create the corresponding scan state. auto* arrowTable = dynamic_cast(tableInfo.table); - auto* parquetTable = dynamic_cast(tableInfo.table); + auto* iceDiskTable = dynamic_cast(tableInfo.table); auto* foreignTable = dynamic_cast(tableInfo.table); if (arrowTable) { scanState = std::make_unique(*MemoryManager::Get(*clientContext), boundNodeIDVector, outVectors, nbrNodeIDVector->state); - } else if (parquetTable) { + } else if (iceDiskTable) { scanState = - std::make_unique(*MemoryManager::Get(*clientContext), + std::make_unique(*MemoryManager::Get(*clientContext), boundNodeIDVector, outVectors, nbrNodeIDVector->state); } else if (foreignTable) { scanState = diff --git a/src/storage/storage_manager.cpp b/src/storage/storage_manager.cpp index 82a22dd750..5acb66548e 100644 --- a/src/storage/storage_manager.cpp +++ b/src/storage/storage_manager.cpp @@ -4,6 +4,7 @@ #include "catalog/catalog_entry/node_table_catalog_entry.h" #include "catalog/catalog_entry/rel_group_catalog_entry.h" #include "common/arrow/arrow.h" +#include "common/constants.h" #include "common/file_system/virtual_file_system.h" #include "common/random_engine.h" #include "common/serializer/in_mem_file_writer.h" @@ -19,9 +20,9 @@ #include "storage/table/arrow_rel_table.h" #include "storage/table/arrow_table_support.h" #include "storage/table/foreign_rel_table.h" +#include "storage/table/ice_disk_node_table.h" +#include "storage/table/ice_disk_rel_table.h" #include "storage/table/node_table.h" -#include "storage/table/parquet_node_table.h" -#include "storage/table/parquet_rel_table.h" #include "storage/table/rel_table.h" #include "storage/wal/wal_replayer.h" #include "transaction/transaction.h" @@ -38,7 +39,8 @@ StorageManager::StorageManager(const std::string& databasePath, bool readOnly, b MemoryManager& memoryManager, bool enableCompression, bool enableDefaultHashIndex, VirtualFileSystem* vfs) : databasePath{databasePath}, readOnly{readOnly}, dataFH{nullptr}, memoryManager{memoryManager}, - enableCompression{enableCompression}, enableDefaultHashIndex{enableDefaultHashIndex} { + enableCompression{enableCompression}, enableDefaultHashIndex{enableDefaultHashIndex}, + vfs_{vfs} { wal = std::make_unique(databasePath, readOnly, enableChecksums, vfs); shadowFile = std::make_unique(*memoryManager.getBufferManager(), vfs, this->databasePath); @@ -122,9 +124,9 @@ void StorageManager::createNodeTable(NodeTableCatalogEntry* entry) { tables[entry->getTableID()] = std::make_unique(this, entry, &memoryManager, std::move(schemaCopy), std::move(arraysCopy), arrowId); } else { - // Create parquet-backed node table + // Create icebug-disk-backed node table tables[entry->getTableID()] = - std::make_unique(this, entry, &memoryManager); + std::make_unique(this, entry, &memoryManager); } } else { // Create regular node table @@ -170,8 +172,8 @@ void StorageManager::addRelTable(RelGroupCatalogEntry* entry, const RelTableCata info.nodePair.dstTableID, this, &memoryManager, fromNodeTable, toNodeTable, std::move(schemaCopy), std::move(arraysCopy), arrowId); } else { - // Create parquet-backed rel table - tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, + // Create icebug-disk-backed rel table + tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, info.nodePair.dstTableID, this, &memoryManager); } } else { @@ -437,8 +439,8 @@ void StorageManager::deserialize(main::ClientContext* context, const Catalog* ca ->ptrCast(); tableNameCache[tableID] = tableEntry->getName(); if (!tableEntry->getStorage().empty()) { - // Create parquet-backed node table - tables[tableID] = std::make_unique(this, tableEntry, &memoryManager); + // Create icebug-disk-backed node table + tables[tableID] = std::make_unique(this, tableEntry, &memoryManager); } else { // Create regular node table tables[tableID] = std::make_unique(this, tableEntry, &memoryManager); @@ -465,8 +467,8 @@ void StorageManager::deserialize(main::ClientContext* context, const Catalog* ca RelTableCatalogInfo info = RelTableCatalogInfo::deserialize(deSer); DASSERT(!tables.contains(info.oid)); if (!relGroupEntry->getStorage().empty()) { - // Create parquet-backed rel table - tables[info.oid] = std::make_unique(relGroupEntry, + // Create icebug-disk-backed rel table + tables[info.oid] = std::make_unique(relGroupEntry, info.nodePair.srcTableID, info.nodePair.dstTableID, this, &memoryManager); } else { // Create regular rel table diff --git a/src/storage/table/CMakeLists.txt b/src/storage/table/CMakeLists.txt index 54a76cca23..121a8e319e 100644 --- a/src/storage/table/CMakeLists.txt +++ b/src/storage/table/CMakeLists.txt @@ -28,8 +28,8 @@ add_library(lbug_storage_store node_group_collection.cpp node_table.cpp null_column.cpp - parquet_node_table.cpp - parquet_rel_table.cpp + ice_disk_node_table.cpp + ice_disk_rel_table.cpp rel_table.cpp rel_table_data.cpp string_chunk_data.cpp diff --git a/src/storage/table/arrow_rel_table.cpp b/src/storage/table/arrow_rel_table.cpp index aa218e87fb..befaa29a20 100644 --- a/src/storage/table/arrow_rel_table.cpp +++ b/src/storage/table/arrow_rel_table.cpp @@ -40,7 +40,7 @@ static int64_t findColumnIdx(const ArrowSchemaWrapper& schema, const std::string void ArrowRelTableScanState::setToTable(const transaction::Transaction* transaction, Table* table_, std::vector columnIDs_, std::vector columnPredicateSets_, RelDataDirection direction_) { - // Same behavior as ParquetRelTable: no local table for external data sources. + // Same behavior as IceDiskRelTable: no local table for external data sources. TableScanState::setToTable(transaction, table_, std::move(columnIDs_), std::move(columnPredicateSets_)); columns.resize(columnIDs.size()); diff --git a/src/storage/table/parquet_node_table.cpp b/src/storage/table/ice_disk_node_table.cpp similarity index 73% rename from src/storage/table/parquet_node_table.cpp rename to src/storage/table/ice_disk_node_table.cpp index b8178cccd6..be46195a3a 100644 --- a/src/storage/table/parquet_node_table.cpp +++ b/src/storage/table/ice_disk_node_table.cpp @@ -1,5 +1,6 @@ -#include "storage/table/parquet_node_table.h" +#include "storage/table/ice_disk_node_table.h" +#include #include #include "catalog/catalog_entry/node_table_catalog_entry.h" @@ -13,6 +14,7 @@ #include "storage/storage_manager.h" #include "storage/storage_utils.h" #include "storage/table/column.h" +#include "storage/table/ice_disk_utils.h" #include "transaction/transaction.h" using namespace lbug::catalog; @@ -23,27 +25,27 @@ using namespace lbug::transaction; namespace lbug { namespace storage { -ParquetNodeTable::ParquetNodeTable(const StorageManager* storageManager, +IceDiskNodeTable::IceDiskNodeTable(const StorageManager* storageManager, const NodeTableCatalogEntry* nodeTableEntry, MemoryManager* memoryManager) : ColumnarNodeTableBase{storageManager, nodeTableEntry, memoryManager, - std::make_unique()} { - std::string prefix = nodeTableEntry->getStorage(); - if (prefix.empty()) { - throw RuntimeException("Parquet file prefix is empty for parquet-backed node table"); - } - - // Use base class helper to construct storage path - parquetFilePath = constructStoragePath(prefix, ".parquet"); + std::make_unique()} { + auto file = IceDiskUtils::constructNodeTablePath( + IceDiskUtils::getBasePath(nodeTableEntry->getStorage()), nodeTableEntry->getName(), + ".parquet"); + const auto dbDir = + std::filesystem::path(storageManager->getDatabasePath()).parent_path().string(); + IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, file); + parquetFilePath = file; } -void ParquetNodeTable::initializeScanCoordination(const transaction::Transaction* transaction) { - auto parquetScanSharedState = - static_cast(tableScanSharedState.get()); +void IceDiskNodeTable::initializeScanCoordination(const transaction::Transaction* transaction) { + auto iceDiskScanSharedState = + static_cast(tableScanSharedState.get()); auto numBatches = getNumBatches(transaction); - parquetScanSharedState->reset(numBatches); + iceDiskScanSharedState->reset(numBatches); } -void ParquetNodeTable::initScanState(Transaction* transaction, TableScanState& scanState, +void IceDiskNodeTable::initScanState(Transaction* transaction, TableScanState& scanState, [[maybe_unused]] bool resetCachedBoundNodeSelVec) const { // Set up the scan state similar to how NodeTable does it auto& nodeScanState = scanState.cast(); @@ -51,19 +53,19 @@ void ParquetNodeTable::initScanState(Transaction* transaction, TableScanState& s // Note: Don't set nodeGroupIdx here - it's set by the morsel-driven parallelism system - auto& parquetNodeScanState = static_cast(nodeScanState); + auto& iceDiskScanState = static_cast(nodeScanState); // Reset scan state for each scan to allow multiple scans of the same table in one query - parquetNodeScanState.dataRead = false; - parquetNodeScanState.allData.clear(); - parquetNodeScanState.totalRows = 0; - parquetNodeScanState.nextRowToDistribute = 0; + iceDiskScanState.dataRead = false; + iceDiskScanState.allData.clear(); + iceDiskScanState.totalRows = 0; + iceDiskScanState.nextRowToDistribute = 0; // Reset scan completion flag for this scan state - parquetNodeScanState.scanCompleted = false; + iceDiskScanState.scanCompleted = false; // Each scan state gets its own parquet reader for thread safety - if (!parquetNodeScanState.initialized) { + if (!iceDiskScanState.initialized) { auto context = transaction->getClientContext(); if (!context) { throw RuntimeException("Invalid client context for parquet scan state initialization"); @@ -72,9 +74,9 @@ void ParquetNodeTable::initScanState(Transaction* transaction, TableScanState& s std::vector columnSkips; try { auto resolvedPath = VirtualFileSystem::resolvePath(context, parquetFilePath); - parquetNodeScanState.parquetReader = + iceDiskScanState.parquetReader = std::make_unique(resolvedPath, columnSkips, context); - parquetNodeScanState.initialized = true; + iceDiskScanState.initialized = true; } catch (const std::exception& e) { throw RuntimeException("Failed to initialize parquet reader for file '" + parquetFilePath + "': " + e.what()); @@ -82,13 +84,13 @@ void ParquetNodeTable::initScanState(Transaction* transaction, TableScanState& s } // Set nodeGroupIdx to invalid initially - will be assigned by getNextBatch - parquetNodeScanState.nodeGroupIdx = INVALID_NODE_GROUP_IDX; + iceDiskScanState.nodeGroupIdx = INVALID_NODE_GROUP_IDX; // Initialize scan state for the current row group (assigned via shared state) - initParquetScanForRowGroup(transaction, parquetNodeScanState); + initParquetScanForRowGroup(transaction, iceDiskScanState); } -common::node_group_idx_t ParquetNodeTable::getNumBatches(const Transaction* transaction) const { +common::node_group_idx_t IceDiskNodeTable::getNumBatches(const Transaction* transaction) const { auto context = transaction->getClientContext(); if (!context) { return 1; @@ -104,8 +106,8 @@ common::node_group_idx_t ParquetNodeTable::getNumBatches(const Transaction* tran } } -void ParquetNodeTable::initParquetScanForRowGroup(Transaction* transaction, - ParquetNodeTableScanState& scanState) const { +void IceDiskNodeTable::initParquetScanForRowGroup(Transaction* transaction, + IceDiskNodeTableScanState& iceDiskScanState) const { auto context = transaction->getClientContext(); if (!context) { return; @@ -117,60 +119,62 @@ void ParquetNodeTable::initParquetScanForRowGroup(Transaction* transaction, } // Defensive check: ensure parquet reader exists - if (!scanState.parquetReader) { + if (!iceDiskScanState.parquetReader) { return; } // Defensive check: ensure parquet scan state exists - if (!scanState.parquetScanState) { + if (!iceDiskScanState.parquetScanState) { return; } std::vector groupsToRead; // Use shared state to get the next available row group for this scan state - if (scanState.nodeGroupIdx == INVALID_NODE_GROUP_IDX) { + if (iceDiskScanState.nodeGroupIdx == INVALID_NODE_GROUP_IDX) { common::node_group_idx_t assignedRowGroup; - if (dynamic_cast(tableScanSharedState.get()) + if (dynamic_cast(tableScanSharedState.get()) ->getNextBatch(assignedRowGroup)) { - scanState.nodeGroupIdx = assignedRowGroup; + iceDiskScanState.nodeGroupIdx = assignedRowGroup; groupsToRead.push_back(assignedRowGroup); } else { // No more row groups available - mark scan as completed - scanState.scanCompleted = true; + iceDiskScanState.scanCompleted = true; // Still need to initialize the scan state with empty groups so reader is in valid state - scanState.parquetReader->initializeScan(*scanState.parquetScanState, groupsToRead, vfs); + iceDiskScanState.parquetReader->initializeScan(*iceDiskScanState.parquetScanState, + groupsToRead, vfs); return; } } else { // Row group already assigned (e.g., by external morsel system or re-initialization) - groupsToRead.push_back(scanState.nodeGroupIdx); + groupsToRead.push_back(iceDiskScanState.nodeGroupIdx); } // Re-initialize scan for the specific row groups // Note: initializeScan can be called multiple times; the first call populates column metadata - scanState.parquetReader->initializeScan(*scanState.parquetScanState, groupsToRead, vfs); + iceDiskScanState.parquetReader->initializeScan(*iceDiskScanState.parquetScanState, groupsToRead, + vfs); } -bool ParquetNodeTable::scanInternal(Transaction* transaction, TableScanState& scanState) { - auto& parquetScanState = static_cast(scanState); +bool IceDiskNodeTable::scanInternal(Transaction* transaction, TableScanState& scanState) { + auto& iceDiskScanState = static_cast(scanState); // Check if this particular scan state has already completed - if (parquetScanState.scanCompleted) { + if (iceDiskScanState.scanCompleted) { return false; } scanState.resetOutVectors(); // Read all data once into scan state - if (!parquetScanState.dataRead) { + if (!iceDiskScanState.dataRead) { // Only the first thread reads the parquet data - if (!parquetScanState.initialized) { + if (!iceDiskScanState.initialized) { return false; } // Create a data chunk for reading parquet data - auto numColumns = parquetScanState.parquetReader->getNumColumns(); + auto numColumns = iceDiskScanState.parquetReader->getNumColumns(); // Defensive check: ensure parquet file has at least one column if (numColumns == 0) { @@ -184,7 +188,7 @@ bool ParquetNodeTable::scanInternal(Transaction* transaction, TableScanState& sc // Always create the data chunk to match the exact number of parquet columns // to prevent crashes in the parquet reader when accessing result vectors for (uint32_t i = 0; i < numColumns; ++i) { - const auto& parquetColumnType = parquetScanState.parquetReader->getColumnType(i); + const auto& parquetColumnType = iceDiskScanState.parquetReader->getColumnType(i); auto columnType = parquetColumnType.copy(); auto vector = std::make_shared(std::move(columnType), MemoryManager::Get(*transaction->getClientContext()), scanState.outState); @@ -192,13 +196,13 @@ bool ParquetNodeTable::scanInternal(Transaction* transaction, TableScanState& sc } // Read from parquet - parquetScanState.parquetReader->scan(*parquetScanState.parquetScanState, parquetDataChunk); + iceDiskScanState.parquetReader->scan(*iceDiskScanState.parquetScanState, parquetDataChunk); auto selSize = parquetDataChunk.state->getSelVector().getSelSize(); if (selSize > 0) { - parquetScanState.allData.resize(selSize); + iceDiskScanState.allData.resize(selSize); for (size_t row = 0; row < selSize; ++row) { - parquetScanState.allData[row].resize( + iceDiskScanState.allData[row].resize( scanState.outputVectors .size()); // Use output vector count, not parquet column count @@ -217,7 +221,7 @@ bool ParquetNodeTable::scanInternal(Transaction* transaction, TableScanState& sc // Get parquet column name and find its corresponding column ID std::string parquetColumnName = - parquetScanState.parquetReader->getColumnName(parquetCol); + iceDiskScanState.parquetReader->getColumnName(parquetCol); auto nodeTableEntry = this->nodeTableCatalogEntry; // Check if the column exists first before calling getColumnID @@ -240,44 +244,44 @@ bool ParquetNodeTable::scanInternal(Transaction* transaction, TableScanState& sc // Only copy data if we found a matching output position if (outputCol != INVALID_COLUMN_ID && - outputCol < parquetScanState.allData[row].size()) { + outputCol < iceDiskScanState.allData[row].size()) { // Defensive check: ensure the row index is valid for the source vector if (row >= srcVector.state->getSelVector().getSelSize()) { continue; } if (srcVector.isNull(row)) { - parquetScanState.allData[row][outputCol] = + iceDiskScanState.allData[row][outputCol] = std::make_unique(Value::createNullValue()); } else { - parquetScanState.allData[row][outputCol] = + iceDiskScanState.allData[row][outputCol] = std::make_unique(*srcVector.getAsValue(row)); } } } } - parquetScanState.totalRows = selSize; + iceDiskScanState.totalRows = selSize; } - parquetScanState.dataRead = true; + iceDiskScanState.dataRead = true; } // Now distribute one row to this scan state - if (parquetScanState.nextRowToDistribute >= parquetScanState.totalRows) { - parquetScanState.scanCompleted = true; + if (iceDiskScanState.nextRowToDistribute >= iceDiskScanState.totalRows) { + iceDiskScanState.scanCompleted = true; return false; // No more rows to distribute } - size_t rowIndex = parquetScanState.nextRowToDistribute++; + size_t rowIndex = iceDiskScanState.nextRowToDistribute++; // Copy one row to output vectors // Defensive checks: ensure valid row index and handle empty data gracefully - if (rowIndex >= parquetScanState.allData.size()) { - parquetScanState.scanCompleted = true; + if (rowIndex >= iceDiskScanState.allData.size()) { + iceDiskScanState.scanCompleted = true; return false; } auto numColumns = - std::min(scanState.outputVectors.size(), parquetScanState.allData[rowIndex].size()); + std::min(scanState.outputVectors.size(), iceDiskScanState.allData[rowIndex].size()); for (size_t col = 0; col < numColumns; ++col) { // Defensive check: ensure output vector exists if (col >= scanState.outputVectors.size() || !scanState.outputVectors[col]) { @@ -287,13 +291,13 @@ bool ParquetNodeTable::scanInternal(Transaction* transaction, TableScanState& sc auto& dstVector = *scanState.outputVectors[col]; // Defensive check: ensure value exists for this column - if (col >= parquetScanState.allData[rowIndex].size() || - !parquetScanState.allData[rowIndex][col]) { + if (col >= iceDiskScanState.allData[rowIndex].size() || + !iceDiskScanState.allData[rowIndex][col]) { dstVector.setNull(0, true); continue; } - auto& value = *parquetScanState.allData[rowIndex][col]; + auto& value = *iceDiskScanState.allData[rowIndex][col]; if (value.isNull()) { dstVector.setNull(0, true); @@ -312,7 +316,7 @@ bool ParquetNodeTable::scanInternal(Transaction* transaction, TableScanState& sc return true; } -row_idx_t ParquetNodeTable::getTotalRowCount(const Transaction* transaction) const { +row_idx_t IceDiskNodeTable::getTotalRowCount(const Transaction* transaction) const { const auto cached = cachedRowCount.load(std::memory_order_relaxed); if (cached != INVALID_ROW_IDX) { return cached; @@ -341,12 +345,12 @@ row_idx_t ParquetNodeTable::getTotalRowCount(const Transaction* transaction) con } } -bool ParquetNodeTable::isVisible(const transaction::Transaction* transaction, +bool IceDiskNodeTable::isVisible(const transaction::Transaction* transaction, common::offset_t offset) const { return offset < getTotalRowCount(transaction); } -bool ParquetNodeTable::isVisibleNoLock(const transaction::Transaction* transaction, +bool IceDiskNodeTable::isVisibleNoLock(const transaction::Transaction* transaction, common::offset_t offset) const { return offset < getTotalRowCount(transaction); } diff --git a/src/storage/table/parquet_rel_table.cpp b/src/storage/table/ice_disk_rel_table.cpp similarity index 75% rename from src/storage/table/parquet_rel_table.cpp rename to src/storage/table/ice_disk_rel_table.cpp index 7237716d54..01f3324777 100644 --- a/src/storage/table/parquet_rel_table.cpp +++ b/src/storage/table/ice_disk_rel_table.cpp @@ -1,4 +1,6 @@ -#include "storage/table/parquet_rel_table.h" +#include "storage/table/ice_disk_rel_table.h" + +#include #include "catalog/catalog_entry/rel_group_catalog_entry.h" #include "common/data_chunk/sel_vector.h" @@ -7,6 +9,7 @@ #include "main/client_context.h" #include "processor/operator/persistent/reader/parquet/parquet_reader.h" #include "storage/storage_manager.h" +#include "storage/table/ice_disk_utils.h" #include "transaction/transaction.h" using namespace lbug::catalog; @@ -17,7 +20,7 @@ using namespace lbug::transaction; namespace lbug { namespace storage { -void ParquetRelTableScanState::setToTable(const Transaction* transaction, Table* table_, +void IceDiskRelTableScanState::setToTable(const Transaction* transaction, Table* table_, std::vector columnIDs_, std::vector columnPredicateSets_, RelDataDirection direction_) { // Call base class implementation but skip local table setup @@ -36,47 +39,48 @@ void ParquetRelTableScanState::setToTable(const Transaction* transaction, Table* csrOffsetColumn = table->cast().getCSROffsetColumn(direction); csrLengthColumn = table->cast().getCSRLengthColumn(direction); nodeGroupIdx = INVALID_NODE_GROUP_IDX; - // ParquetRelTable does not support local storage, so we skip the local table initialization + // IceDiskRelTable does not support local storage, so we skip the local table initialization } -ParquetRelTable::ParquetRelTable(RelGroupCatalogEntry* relGroupEntry, table_id_t fromTableID, +IceDiskRelTable::IceDiskRelTable(RelGroupCatalogEntry* relGroupEntry, table_id_t fromTableID, table_id_t toTableID, const StorageManager* storageManager, MemoryManager* memoryManager) : ColumnarRelTableBase{relGroupEntry, fromTableID, toTableID, storageManager, memoryManager} { - std::string storage = relGroupEntry->getStorage(); - if (storage.empty()) { - throw RuntimeException("Parquet file path is empty for parquet-backed rel table"); - } + const auto base = IceDiskUtils::getBasePath(relGroupEntry->getStorage()); + auto paths = IceDiskUtils::constructCSRPaths(base, relGroupEntry->getName(), ".parquet"); + + const auto dbDir = + std::filesystem::path(storageManager->getDatabasePath()).parent_path().string(); + IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, paths.indices); + IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, paths.indptr); - // Use base class helper to construct CSR file paths - auto paths = constructCSRPaths(storage, ".parquet"); indicesFilePath = paths.indices; indptrFilePath = paths.indptr; } -void ParquetRelTable::initScanState(Transaction* transaction, TableScanState& scanState, +void IceDiskRelTable::initScanState(Transaction* transaction, TableScanState& scanState, bool resetCachedBoundNodeSelVec) const { - // For parquet tables, we create our own scan state + // For tables, we create our own scan state auto& relScanState = scanState.cast(); relScanState.source = TableScanSource::COMMITTED; relScanState.nodeGroup = nullptr; relScanState.nodeGroupIdx = INVALID_NODE_GROUP_IDX; // Initialize ParquetReaders for this scan state (per-thread) - auto& parquetRelScanState = static_cast(relScanState); + auto& iceDiskScanState = static_cast(relScanState); // Initialize readers if not already done for this scan state - if (!parquetRelScanState.indicesReader) { + if (!iceDiskScanState.indicesReader) { std::vector columnSkips; // Read all columns auto context = transaction->getClientContext(); auto resolvedPath = VirtualFileSystem::resolvePath(context, indicesFilePath); - parquetRelScanState.indicesReader = + iceDiskScanState.indicesReader = std::make_unique(resolvedPath, columnSkips, context); } - if (!indptrFilePath.empty() && !parquetRelScanState.indptrReader) { + if (!indptrFilePath.empty() && !iceDiskScanState.indptrReader) { std::vector columnSkips; // Read all columns auto context = transaction->getClientContext(); auto resolvedPath = VirtualFileSystem::resolvePath(context, indptrFilePath); - parquetRelScanState.indptrReader = + iceDiskScanState.indptrReader = std::make_unique(resolvedPath, columnSkips, context); } @@ -103,13 +107,12 @@ void ParquetRelTable::initScanState(Transaction* transaction, TableScanState& sc // Initialize row group ranges for morsel-driven parallelism // For now, assign all row groups to this scan state (will be partitioned by the scan operator) - parquetRelScanState.currentRowGroup = 0; - parquetRelScanState.endRowGroup = parquetRelScanState.indicesReader ? - parquetRelScanState.indicesReader->getNumRowGroups() : - 0; + iceDiskScanState.currentRowGroup = 0; + iceDiskScanState.endRowGroup = + iceDiskScanState.indicesReader ? iceDiskScanState.indicesReader->getNumRowGroups() : 0; } -void ParquetRelTable::initializeParquetReaders(Transaction* transaction) const { +void IceDiskRelTable::initializeParquetReaders(Transaction* transaction) const { if (!indicesReader) { std::lock_guard lock(parquetReaderMutex); if (!indicesReader) { @@ -121,7 +124,7 @@ void ParquetRelTable::initializeParquetReaders(Transaction* transaction) const { } } -void ParquetRelTable::initializeIndptrReader(Transaction* transaction) const { +void IceDiskRelTable::initializeIndptrReader(Transaction* transaction) const { if (!indptrFilePath.empty() && !indptrReader) { std::lock_guard lock(parquetReaderMutex); if (!indptrReader) { @@ -133,7 +136,7 @@ void ParquetRelTable::initializeIndptrReader(Transaction* transaction) const { } } -void ParquetRelTable::loadIndptrData(Transaction* transaction) const { +void IceDiskRelTable::loadIndptrData(Transaction* transaction) const { if (indptrData.empty() && !indptrFilePath.empty()) { std::lock_guard lock(indptrDataMutex); if (indptrData.empty()) { @@ -186,11 +189,11 @@ void ParquetRelTable::loadIndptrData(Transaction* transaction) const { } } -bool ParquetRelTable::scanInternal(Transaction* transaction, TableScanState& scanState) { +bool IceDiskRelTable::scanInternal(Transaction* transaction, TableScanState& scanState) { auto& relScanState = scanState.cast(); - // Get the ParquetRelTableScanState - auto& parquetRelScanState = static_cast(relScanState); + // Get the IceDiskRelTableScanState + auto& iceDiskScanState = static_cast(relScanState); // Load shared indptr data - thread-safe to read if (!indptrFilePath.empty()) { @@ -199,49 +202,49 @@ bool ParquetRelTable::scanInternal(Transaction* transaction, TableScanState& sca // True morsel-driven parallelism: each scan state processes its assigned row groups // Process all row groups assigned to this scan state, collecting relationships for bound nodes - return scanInternalByRowGroups(transaction, parquetRelScanState); + return scanInternalByRowGroups(transaction, iceDiskScanState); } -bool ParquetRelTable::scanInternalByRowGroups(Transaction* transaction, - ParquetRelTableScanState& parquetRelScanState) { +bool IceDiskRelTable::scanInternalByRowGroups(Transaction* transaction, + IceDiskRelTableScanState& iceDiskScanState) { // True morsel-driven parallelism: process assigned row groups and collect relationships for // bound nodes // Check if we have any row groups left to process - if (parquetRelScanState.currentRowGroup >= parquetRelScanState.endRowGroup) { + if (iceDiskScanState.currentRowGroup >= iceDiskScanState.endRowGroup) { // No more row groups to process - parquetRelScanState.outState->getSelVectorUnsafe().setToFiltered(0); + iceDiskScanState.outState->getSelVectorUnsafe().setToFiltered(0); return false; } // Process the current row group - std::vector rowGroupsToProcess = {parquetRelScanState.currentRowGroup}; + std::vector rowGroupsToProcess = {iceDiskScanState.currentRowGroup}; // Create a set of bound node IDs for fast lookup std::unordered_map boundNodeOffsets; - for (size_t i = 0; i < parquetRelScanState.cachedBoundNodeSelVector.getSelSize(); ++i) { - common::sel_t boundNodeIdx = parquetRelScanState.cachedBoundNodeSelVector[i]; - const auto boundNodeID = parquetRelScanState.nodeIDVector->getValue(boundNodeIdx); + for (size_t i = 0; i < iceDiskScanState.cachedBoundNodeSelVector.getSelSize(); ++i) { + common::sel_t boundNodeIdx = iceDiskScanState.cachedBoundNodeSelVector[i]; + const auto boundNodeID = iceDiskScanState.nodeIDVector->getValue(boundNodeIdx); boundNodeOffsets.insert({boundNodeID.offset, boundNodeIdx}); } // Scan the current row group and collect relationships for bound nodes - bool hasData = scanRowGroupForBoundNodes(transaction, parquetRelScanState, rowGroupsToProcess, + bool hasData = scanRowGroupForBoundNodes(transaction, iceDiskScanState, rowGroupsToProcess, boundNodeOffsets); // Move to next row group for next call - parquetRelScanState.currentRowGroup++; + iceDiskScanState.currentRowGroup++; return hasData; } -common::offset_t ParquetRelTable::findSourceNodeForRow(common::offset_t globalRowIdx) const { +common::offset_t IceDiskRelTable::findSourceNodeForRow(common::offset_t globalRowIdx) const { // Use base class helper for binary search return findSourceNodeForRowInternal(globalRowIdx, indptrData); } -bool ParquetRelTable::scanRowGroupForBoundNodes(Transaction* transaction, - ParquetRelTableScanState& parquetRelScanState, const std::vector& rowGroupsToProcess, +bool IceDiskRelTable::scanRowGroupForBoundNodes(Transaction* transaction, + IceDiskRelTableScanState& iceDiskScanState, const std::vector& rowGroupsToProcess, const std::unordered_map& boundNodeOffsets) { // Initialize readers if needed @@ -250,24 +253,24 @@ bool ParquetRelTable::scanRowGroupForBoundNodes(Transaction* transaction, // Initialize scan state for the assigned row groups auto context = transaction->getClientContext(); auto vfs = VirtualFileSystem::GetUnsafe(*context); - parquetRelScanState.indicesReader->initializeScan(*parquetRelScanState.parquetScanState, + iceDiskScanState.indicesReader->initializeScan(*iceDiskScanState.parquetScanState, rowGroupsToProcess, vfs); // Create DataChunk matching the indices parquet file schema - auto numIndicesColumns = parquetRelScanState.indicesReader->getNumColumns(); + auto numIndicesColumns = iceDiskScanState.indicesReader->getNumColumns(); DataChunk indicesChunk(numIndicesColumns); // Insert value vectors for all columns in the parquet file auto memoryManager = MemoryManager::Get(*context); for (uint32_t colIdx = 0; colIdx < numIndicesColumns; ++colIdx) { - const auto& columnTypeRef = parquetRelScanState.indicesReader->getColumnType(colIdx); + const auto& columnTypeRef = iceDiskScanState.indicesReader->getColumnType(colIdx); auto columnType = columnTypeRef.copy(); auto vector = std::make_shared(std::move(columnType), memoryManager); indicesChunk.insert(colIdx, vector); } // Scan the row groups and collect relationships for bound nodes. - const auto isFwd = parquetRelScanState.direction != RelDataDirection::BWD; + const auto isFwd = iceDiskScanState.direction != RelDataDirection::BWD; uint64_t totalRowsCollected = 0; const uint64_t maxRowsPerCall = DEFAULT_VECTOR_CAPACITY; uint64_t currentGlobalRowIdx = 0; @@ -277,14 +280,14 @@ bool ParquetRelTable::scanRowGroupForBoundNodes(Transaction* transaction, // Calculate the starting global row index for the first row group if (!rowGroupsToProcess.empty()) { - auto metadata = parquetRelScanState.indicesReader->getMetadata(); + auto metadata = iceDiskScanState.indicesReader->getMetadata(); for (uint64_t rgIdx = 0; rgIdx < rowGroupsToProcess[0]; ++rgIdx) { currentGlobalRowIdx += metadata->row_groups[rgIdx].num_rows; } } while (totalRowsCollected < maxRowsPerCall && - parquetRelScanState.indicesReader->scanInternal(*parquetRelScanState.parquetScanState, + iceDiskScanState.indicesReader->scanInternal(*iceDiskScanState.parquetScanState, indicesChunk)) { auto selSize = indicesChunk.state->getSelVector().getSelSize(); @@ -319,31 +322,31 @@ bool ParquetRelTable::scanRowGroupForBoundNodes(Transaction* transaction, auto nbrNodeID = internalID_t(nbrOffset, nbrTableID); // outputVectors[0] is the neighbor node ID, if requested. - if (!parquetRelScanState.outputVectors.empty()) { - parquetRelScanState.outputVectors[0]->setValue(totalRowsCollected, nbrNodeID); + if (!iceDiskScanState.outputVectors.empty()) { + iceDiskScanState.outputVectors[0]->setValue(totalRowsCollected, nbrNodeID); } // Copy edge properties to output vectors. // Catalog col IDs: NBR_ID=0, REL_ID=1 (virtual), user props=2,3,... // Parquet cols: target=0, user props=1,2,... // So parquet_col = catalog_col_id - 1 for user properties. - for (uint64_t outCol = 1; outCol < parquetRelScanState.outputVectors.size(); ++outCol) { - if (outCol >= parquetRelScanState.columnIDs.size()) { + for (uint64_t outCol = 1; outCol < iceDiskScanState.outputVectors.size(); ++outCol) { + if (outCol >= iceDiskScanState.columnIDs.size()) { continue; } - const auto colID = parquetRelScanState.columnIDs[outCol]; + const auto colID = iceDiskScanState.columnIDs[outCol]; if (colID == INVALID_COLUMN_ID || colID == ROW_IDX_COLUMN_ID || colID == NBR_ID_COLUMN_ID) { continue; } if (colID == REL_ID_COLUMN_ID) { // REL_ID is not stored in parquet; synthesize from the global row index. - parquetRelScanState.outputVectors[outCol]->setValue( + iceDiskScanState.outputVectors[outCol]->setValue( totalRowsCollected, internalID_t{currentGlobalRowIdx, getTableID()}); continue; } - parquetRelScanState.outputVectors[outCol]->copyFromVectorData(totalRowsCollected, + iceDiskScanState.outputVectors[outCol]->copyFromVectorData(totalRowsCollected, &indicesChunk.getValueVector(colID - 1), i); } @@ -353,22 +356,22 @@ bool ParquetRelTable::scanRowGroupForBoundNodes(Transaction* transaction, // Set up the output state if (totalRowsCollected > 0) { - auto& selVector = parquetRelScanState.outState->getSelVectorUnsafe(); + auto& selVector = iceDiskScanState.outState->getSelVectorUnsafe(); selVector.setToFiltered(totalRowsCollected); for (uint64_t i = 0; i < totalRowsCollected; ++i) { selVector[i] = i; } - parquetRelScanState.setNodeIDVectorToFlat(activeBoundSelPos); + iceDiskScanState.setNodeIDVectorToFlat(activeBoundSelPos); return true; } else { // No data found - parquetRelScanState.outState->getSelVectorUnsafe().setToFiltered(0); + iceDiskScanState.outState->getSelVectorUnsafe().setToFiltered(0); return false; } } -row_idx_t ParquetRelTable::getTotalRowCount(const Transaction* transaction) const { +row_idx_t IceDiskRelTable::getTotalRowCount(const Transaction* transaction) const { initializeParquetReaders(const_cast(transaction)); if (!indicesReader) { return 0; diff --git a/test/include/test_runner/test_group.h b/test/include/test_runner/test_group.h index b0ba4663cd..d0263a7a48 100644 --- a/test/include/test_runner/test_group.h +++ b/test/include/test_runner/test_group.h @@ -116,7 +116,7 @@ struct TestGroup { LBUG, JSON, CSV_TO_JSON, - GRAPH_STD + ICEBUG_DISK }; DatasetType datasetType; diff --git a/test/runner/e2e_test.cpp b/test/runner/e2e_test.cpp index 5c78c5d9fc..2a26f9bf5d 100644 --- a/test/runner/e2e_test.cpp +++ b/test/runner/e2e_test.cpp @@ -39,8 +39,8 @@ class EndToEndTest final : public DBTest { } createDB(checkpointWaitTimeout); createConns(connNames); - if (datasetType == TestGroup::DatasetType::GRAPH_STD) { - // For GRAPH_STD, only run schema.cypher (which contains WITH storage = ... clauses) + if (datasetType == TestGroup::DatasetType::ICEBUG_DISK) { + // For ICEBUG_DISK, only run schema.cypher (which contains WITH storage = ... clauses) // No copy.cypher needed as data is in external parquet files lbug::main::Connection* connection = conn ? conn.get() : (connMap.begin()->second).get(); diff --git a/test/test_files/demo_db/demo_db_graph_std.test b/test/test_files/demo_db/demo_db_icebug_disk.test similarity index 98% rename from test/test_files/demo_db/demo_db_graph_std.test rename to test/test_files/demo_db/demo_db_icebug_disk.test index d22cca98ab..5e3adfe00c 100644 --- a/test/test_files/demo_db/demo_db_graph_std.test +++ b/test/test_files/demo_db/demo_db_icebug_disk.test @@ -1,8 +1,8 @@ --DATASET GRAPH-STD demo-db/icebug-disk +-DATASET ICEBUG-DISK demo-db/icebug-disk -- --CASE DemoDBGraphStdTest +-CASE DemoDBIcebugDiskTest -LOG MatchUserLivesInCity -STATEMENT MATCH (u:user)-[l:livesin]->(c:city) RETURN u.name, u.age, c.name; diff --git a/test/test_helper/test_helper.cpp b/test/test_helper/test_helper.cpp index 6155b1faed..239c03fe1c 100644 --- a/test/test_helper/test_helper.cpp +++ b/test/test_helper/test_helper.cpp @@ -45,6 +45,8 @@ void TestHelper::executeScript(const std::string& cypherScript, Connection& conn } std::string line; while (getline(file, line)) { + // replace single quote with double + std::replace(line.begin(), line.end(), '\'', '"'); // If this is a COPY statement, we need to append the LBUG_ROOT_DIRECTORY to the csv // file path. There maybe multiple csv files in the line, so we need to find all of them. std::vector csvFilePaths; @@ -86,16 +88,17 @@ void TestHelper::executeScript(const std::string& cypherScript, Connection& conn fullPath = normalizePathForCypher(std::move(fullPath)); line.replace(line.find(csvFilePath), csvFilePath.length(), fullPath); } - // Also handle storage = 'path' for parquet tables + + // Also handle storage = 'path' for icebug-disk tables std::vector storagePaths; size_t storageIndex = 0; while (true) { - size_t start = line.find("storage = '", storageIndex); + size_t start = line.find("storage = \"", storageIndex); if (start == std::string::npos) { break; } - start += 11; // length of "storage = '" - size_t end = line.find("'", start); + start += 11; // length of "storage = \"" + size_t end = line.find("\"", start); if (end == std::string::npos) { break; } @@ -104,6 +107,35 @@ void TestHelper::executeScript(const std::string& cypherScript, Connection& conn storageIndex = end + 1; } for (auto& storagePath : storagePaths) { + static constexpr std::string_view iceBugPrefix = "icebug-disk"; + + if (storagePath.starts_with(iceBugPrefix)) { + // Strip "icebug-disk" prefix and optional ':' separator. + std::string basePath = storagePath.substr(iceBugPrefix.size()); + if (!basePath.empty() && basePath[0] == ':') { + basePath = basePath.substr(1); + } + // Resolve empty or relative paths relative to the schema's directory. + // Absolute paths and object-store URLs (contain "://") are left unchanged. + if (basePath.empty()) { + basePath = normalizePathForCypher(cypherDir.string()); + } else if (basePath.find("://") == std::string::npos && + std::filesystem::path(basePath).is_relative()) { + basePath = normalizePathForCypher((cypherDir / basePath).string()); + } + std::string resolvedStorage = std::string(iceBugPrefix) + ":" + basePath; + size_t pos = line.find(storagePath); + if (pos != std::string::npos) { + line.replace(pos, storagePath.length(), resolvedStorage); + } + continue; + } + + if (storagePath.find("://") != std::string::npos) { + // Non-icebug-disk URI — do not modify. + continue; + } + auto fullPath = storagePath; if (std::filesystem::path(storagePath).is_relative()) { if (std::filesystem::path(storagePath).parent_path().empty()) { diff --git a/test/test_runner/test_parser.cpp b/test/test_runner/test_parser.cpp index dea0ef047d..3d4f4cb082 100644 --- a/test/test_runner/test_parser.cpp +++ b/test/test_runner/test_parser.cpp @@ -88,8 +88,8 @@ void TestParser::extractDataset() { testGroup->datasetType = TestGroup::DatasetType::JSON; testGroup->dataset = currentToken.params[2]; } - } else if (datasetType == "GRAPH-STD") { - testGroup->datasetType = TestGroup::DatasetType::GRAPH_STD; + } else if (datasetType == "ICEBUG-DISK") { + testGroup->datasetType = TestGroup::DatasetType::ICEBUG_DISK; testGroup->dataset = currentToken.params[2]; } else { throw TestException( From 13efd642ee313403308b6f0b8408607e0578979d Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Mon, 11 May 2026 10:06:33 +0530 Subject: [PATCH 02/10] add unit tests --- src/binder/bind/bind_ddl.cpp | 3 +- test/storage/CMakeLists.txt | 1 + test/storage/ice_disk_utils_test.cpp | 181 ++++++++++++++++++ .../ice_disk/ice_disk_invalid_storage.test | 34 ++++ .../ice_disk/ice_disk_mix_tables.test | 31 +++ 5 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 test/storage/ice_disk_utils_test.cpp create mode 100644 test/test_files/ice_disk/ice_disk_invalid_storage.test create mode 100644 test/test_files/ice_disk/ice_disk_mix_tables.test diff --git a/src/binder/bind/bind_ddl.cpp b/src/binder/bind/bind_ddl.cpp index 4026f09fd8..26e3822c71 100644 --- a/src/binder/bind/bind_ddl.cpp +++ b/src/binder/bind/bind_ddl.cpp @@ -227,9 +227,10 @@ BoundCreateTableInfo Binder::bindCreateRelTableGroupInfo(const CreateTableInfo* if (!storage.empty()) { auto dotPos = storage.find('.'); // Check if storage is database.table format by verifying the attached database exists + // 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 (dotPos != std::string::npos) { + if (!TableOptionConstants::isIceBugDiskStorage(storage) && dotPos != std::string::npos) { std::string dbName = storage.substr(0, dotPos); std::string tableName = storage.substr(dotPos + 1); if (!dbName.empty()) { diff --git a/test/storage/CMakeLists.txt b/test/storage/CMakeLists.txt index e63f929d90..83efdeabba 100644 --- a/test/storage/CMakeLists.txt +++ b/test/storage/CMakeLists.txt @@ -7,5 +7,6 @@ add_lbug_test(rel_tests rel_scan_test.cpp rel_delete_test.cpp) add_lbug_test(node_update_test node_update_test.cpp) add_lbug_test(detach_delete_test detach_delete_test.cpp) add_lbug_test(storage_utils_test storage_utils_test.cpp) +add_lbug_test(ice_disk_utils_test ice_disk_utils_test.cpp) target_include_directories(compression_test PRIVATE ${PROJECT_SOURCE_DIR}/third_party/alp/include) diff --git a/test/storage/ice_disk_utils_test.cpp b/test/storage/ice_disk_utils_test.cpp new file mode 100644 index 0000000000..f0290a515b --- /dev/null +++ b/test/storage/ice_disk_utils_test.cpp @@ -0,0 +1,181 @@ +#include +#include + +#include "common/exception/copy.h" +#include "common/exception/io.h" +#include "common/exception/runtime.h" +#include "common/file_system/virtual_file_system.h" +#include "gtest/gtest.h" +#include "storage/table/ice_disk_utils.h" +#include "test_helper/test_helper.h" + +using namespace lbug::common; +using namespace lbug::storage; +using namespace lbug::testing; + +static const std::string FIXTURES_DIR = + TestHelper::appendLbugRootPath("dataset/ice-disk-test/fixtures"); +static const std::string DEMO_DB_ICEBUG_DISK = + TestHelper::appendLbugRootPath("dataset/demo-db/icebug-disk"); + +// ───────────────────────────────────────────────────────────── +// getBasePath +// ───────────────────────────────────────────────────────────── +TEST(IceDiskUtils_GetBasePath, EmptyString) { + EXPECT_EQ("", IceDiskUtils::getBasePath("")); +} + +TEST(IceDiskUtils_GetBasePath, PrefixOnly) { + EXPECT_EQ("", IceDiskUtils::getBasePath("icebug-disk")); +} + +TEST(IceDiskUtils_GetBasePath, PrefixWithColonOnly) { + EXPECT_EQ("", IceDiskUtils::getBasePath("icebug-disk:")); +} + +TEST(IceDiskUtils_GetBasePath, PrefixWithAbsolutePath) { + EXPECT_EQ("/some/path", IceDiskUtils::getBasePath("icebug-disk:/some/path")); +} + +TEST(IceDiskUtils_GetBasePath, PrefixWithRelativePath) { + EXPECT_EQ("rel/path", IceDiskUtils::getBasePath("icebug-disk:rel/path")); +} + +TEST(IceDiskUtils_GetBasePath, NonIceDiskPrefix) { + EXPECT_EQ("", IceDiskUtils::getBasePath("parquet:/path")); +} + +TEST(IceDiskUtils_GetBasePath, PrefixWithDot) { + EXPECT_EQ(".", IceDiskUtils::getBasePath("icebug-disk:.")); +} + +// ───────────────────────────────────────────────────────────── +// joinPath +// ───────────────────────────────────────────────────────────── +TEST(IceDiskUtils_JoinPath, EmptyBase) { + EXPECT_EQ("file.parquet", IceDiskUtils::joinPath("", "file.parquet")); +} + +TEST(IceDiskUtils_JoinPath, BaseWithoutTrailingSlash) { + EXPECT_EQ("/base/file.parquet", IceDiskUtils::joinPath("/base", "file.parquet")); +} + +TEST(IceDiskUtils_JoinPath, BaseWithTrailingSlash) { + EXPECT_EQ("/base/file.parquet", IceDiskUtils::joinPath("/base/", "file.parquet")); +} + +TEST(IceDiskUtils_JoinPath, BaseWithBackslash) { + EXPECT_EQ("base\\file.parquet", IceDiskUtils::joinPath("base\\", "file.parquet")); +} + +// ───────────────────────────────────────────────────────────── +// constructNodeTablePath +// ───────────────────────────────────────────────────────────── +TEST(IceDiskUtils_ConstructNodeTablePath, EmptyDir) { + EXPECT_EQ("nodes_city.parquet", IceDiskUtils::constructNodeTablePath("", "city", ".parquet")); +} + +TEST(IceDiskUtils_ConstructNodeTablePath, WithDir) { + EXPECT_EQ("/some/dir/nodes_user.parquet", + IceDiskUtils::constructNodeTablePath("/some/dir", "user", ".parquet")); +} + +// ───────────────────────────────────────────────────────────── +// constructCSRPaths +// ───────────────────────────────────────────────────────────── +TEST(IceDiskUtils_ConstructCSRPaths, EmptyDir) { + auto paths = IceDiskUtils::constructCSRPaths("", "follows", ".parquet"); + EXPECT_EQ("indices_follows.parquet", paths.indices); + EXPECT_EQ("indptr_follows.parquet", paths.indptr); +} + +TEST(IceDiskUtils_ConstructCSRPaths, WithDir) { + auto paths = IceDiskUtils::constructCSRPaths("/some/dir", "knows", ".parquet"); + EXPECT_EQ("/some/dir/indices_knows.parquet", paths.indices); + EXPECT_EQ("/some/dir/indptr_knows.parquet", paths.indptr); +} + +// ───────────────────────────────────────────────────────────── +// resolveIceDiskPath +// ───────────────────────────────────────────────────────────── +TEST(IceDiskUtils_ResolveIceDiskPath, AbsolutePathUnchanged) { + EXPECT_EQ("/abs/path/file.parquet", + IceDiskUtils::resolveIceDiskPath("/abs/path/file.parquet", "/dbdir")); +} + +TEST(IceDiskUtils_ResolveIceDiskPath, RelativePathJoinedWithDbDir) { + auto result = IceDiskUtils::resolveIceDiskPath("rel/file.parquet", "/dbdir"); + // std::filesystem normalizes the path, so check it ends with rel/file.parquet + EXPECT_TRUE(result.find("rel/file.parquet") != std::string::npos); + EXPECT_TRUE(std::filesystem::path(result).is_absolute()); +} + +TEST(IceDiskUtils_ResolveIceDiskPath, EmptyPathUsesDbDir) { + auto result = IceDiskUtils::resolveIceDiskPath("", "/dbdir"); + EXPECT_TRUE(std::filesystem::path(result).is_absolute()); +} + +TEST(IceDiskUtils_ResolveIceDiskPath, DotPathNormalized) { + auto result = IceDiskUtils::resolveIceDiskPath(".", "/dbdir"); + // "." relative to /dbdir should resolve to /dbdir (with possible trailing slash on some impls) + EXPECT_TRUE(result == "/dbdir" || result == "/dbdir/"); +} + +// ───────────────────────────────────────────────────────────── +// checkVersionCompatibility +// ───────────────────────────────────────────────────────────── +class IceDiskCheckVersionTest : public ::testing::Test { +protected: + VirtualFileSystem vfs; + const std::string dbDir = FIXTURES_DIR; +}; + +TEST_F(IceDiskCheckVersionTest, NullVfs) { + EXPECT_THROW(IceDiskUtils::checkVersionCompatibility(nullptr, dbDir, + DEMO_DB_ICEBUG_DISK + "/nodes_person.parquet"), + RuntimeException); +} + +TEST_F(IceDiskCheckVersionTest, FileDoesNotExist) { + EXPECT_THROW(IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + FIXTURES_DIR + "/nodes_nonexistent.parquet"), + IOException); +} + +TEST_F(IceDiskCheckVersionTest, NotAParquetFile) { + EXPECT_THROW(IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + FIXTURES_DIR + "/nodes_notparquet.parquet"), + CopyException); +} + +TEST_F(IceDiskCheckVersionTest, MissingVersionKey) { + try { + IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + FIXTURES_DIR + "/nodes_noversion.parquet"); + FAIL() << "Expected RuntimeException for missing version key"; + } catch (const RuntimeException& e) { + EXPECT_TRUE(std::string(e.what()).find("missing icebug_disk_version") != std::string::npos); + } +} + +TEST_F(IceDiskCheckVersionTest, WrongVersionValue) { + try { + IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + FIXTURES_DIR + "/nodes_wrongversion.parquet"); + FAIL() << "Expected RuntimeException for wrong version"; + } catch (const RuntimeException& e) { + EXPECT_TRUE(std::string(e.what()).find("does not support icebug_disk_version: v99") != + std::string::npos); + } +} + +TEST_F(IceDiskCheckVersionTest, UppercaseVersionSucceeds) { + // "V1" should match "v1" case-insensitively + EXPECT_NO_THROW(IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + FIXTURES_DIR + "/nodes_upperversion.parquet")); +} + +TEST_F(IceDiskCheckVersionTest, ValidV1Succeeds) { + EXPECT_NO_THROW(IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + DEMO_DB_ICEBUG_DISK + "/nodes_user.parquet")); +} diff --git a/test/test_files/ice_disk/ice_disk_invalid_storage.test b/test/test_files/ice_disk/ice_disk_invalid_storage.test new file mode 100644 index 0000000000..be0ea1e929 --- /dev/null +++ b/test/test_files/ice_disk/ice_disk_invalid_storage.test @@ -0,0 +1,34 @@ +-DATASET CSV empty +-- + +-CASE IceDiskNonExistentDirNodeFails + +-LOG IceDiskNonExistentDirMissingParquet +-STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:/nonexistent/dir/that/does/not/exist') +---- error(regex) +.*[Nn]o such file.*|.*[Cc]annot open.*|.*nodes_t\.parquet.* + +-CASE IceDiskURINodeFails + +-LOG IceDiskURIMissingParquet +-STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:https://example.com/path') +---- error(regex) +.*[Nn]o such file.*|.*[Cc]annot open.*|.*nodes_t\.parquet.* + +-CASE IceDiskPrefixOnlyRelFails + +-LOG IceDiskPrefixOnlyRelMissingParquet +-STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') +---- ok +-STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) WITH (STORAGE = 'icebug-disk') +---- error(regex) +.*[Nn]o such file.*|.*[Cc]annot open.*|.*indices_rel\.parquet.* + +-CASE IceDiskPrefixWithColonRelFails + +-LOG IceDiskPrefixColonRelMissingParquet +-STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') +---- ok +-STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) WITH (STORAGE = 'icebug-disk:') +---- error(regex) +.*[Nn]o such file.*|.*[Cc]annot open.*|.*indices_rel\.parquet.* diff --git a/test/test_files/ice_disk/ice_disk_mix_tables.test b/test/test_files/ice_disk/ice_disk_mix_tables.test new file mode 100644 index 0000000000..6c8a97997a --- /dev/null +++ b/test/test_files/ice_disk/ice_disk_mix_tables.test @@ -0,0 +1,31 @@ +-DATASET CSV empty +-- + +-CASE RelIceDiskNodeRegular + +-LOG CreateRegularNodeTable +-STATEMENT CREATE NODE TABLE person(id INT64 PRIMARY KEY, name STRING) +---- ok + +-LOG IceDiskRelWithRegularNodeFails +-STATEMENT CREATE REL TABLE rel(FROM person TO person) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') +---- error +Binder exception: icebug-disk rel tables require both FROM and TO tables to be icebug-disk node tables. + +-CASE NodeIceDiskRelRegular + +-LOG CreateIceDiskNodeTable +-STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') +---- ok + +-LOG RegularRelWithIceDiskNodeFails +-STATEMENT CREATE NODE TABLE person(id INT64 PRIMARY KEY, name STRING) +-STATEMENT CREATE REL TABLE rel(FROM person TO upperversion) +---- error +Binder exception: Rel tables with non-icebug-disk storage do not support icebug-disk node tables as FROM or TO tables. + +-LOG RegularRelWithBothIceDiskNodeFails +-STATEMENT CREATE REL TABLE rel(FROM person TO upperversion) +-STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) +---- error +Binder exception: Rel tables with non-icebug-disk storage do not support icebug-disk node tables as FROM or TO tables. From c3cf83c1ca33a89354964ee27b48a7e133125b01 Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Mon, 11 May 2026 16:04:02 +0530 Subject: [PATCH 03/10] fix storage manager defaulting to ice-disk --- src/include/storage/table/ice_disk_utils.h | 3 --- src/storage/storage_manager.cpp | 13 +++++++++--- .../ice_disk/ice_disk_invalid_storage.test | 12 +++++------ test/test_helper/test_helper.cpp | 20 ------------------- 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/src/include/storage/table/ice_disk_utils.h b/src/include/storage/table/ice_disk_utils.h index cc976ec002..6fd2bd3fbd 100644 --- a/src/include/storage/table/ice_disk_utils.h +++ b/src/include/storage/table/ice_disk_utils.h @@ -26,9 +26,6 @@ class IceDiskUtils { // Parses "icebug-disk", "icebug-disk:", or "icebug-disk:" and returns the path // component. Returns empty string for the first two forms (caller interprets as current dir) static std::string getBasePath(const std::string& storage) { - if (!storage.starts_with(common::TableOptionConstants::ICEBUG_DISK_PREFIX)) { - return ""; - } std::string_view rest = std::string_view(storage).substr( common::TableOptionConstants::ICEBUG_DISK_PREFIX.size()); // Strip the optional ':' separator. diff --git a/src/storage/storage_manager.cpp b/src/storage/storage_manager.cpp index 5acb66548e..6e3d33d4b9 100644 --- a/src/storage/storage_manager.cpp +++ b/src/storage/storage_manager.cpp @@ -123,10 +123,13 @@ void StorageManager::createNodeTable(NodeTableCatalogEntry* entry) { // Create Arrow-backed node table tables[entry->getTableID()] = std::make_unique(this, entry, &memoryManager, std::move(schemaCopy), std::move(arraysCopy), arrowId); - } else { + } else if (TableOptionConstants::isIceBugDiskStorage(entry->getStorage())) { // Create icebug-disk-backed node table tables[entry->getTableID()] = std::make_unique(this, entry, &memoryManager); + } else { + throw common::RuntimeException( + "Unsupported storage option for node table: " + entry->getStorage()); } } else { // Create regular node table @@ -171,10 +174,13 @@ void StorageManager::addRelTable(RelGroupCatalogEntry* entry, const RelTableCata tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, info.nodePair.dstTableID, this, &memoryManager, fromNodeTable, toNodeTable, std::move(schemaCopy), std::move(arraysCopy), arrowId); - } else { + } else if (TableOptionConstants::isIceBugDiskStorage(entry->getStorage())) { // Create icebug-disk-backed rel table tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, info.nodePair.dstTableID, this, &memoryManager); + } else { + throw common::RuntimeException( + "Unsupported storage option for rel table: " + entry->getStorage()); } } else { // Create regular rel table @@ -466,7 +472,8 @@ void StorageManager::deserialize(main::ClientContext* context, const Catalog* ca for (auto k = 0u; k < numInnerRelTables; k++) { RelTableCatalogInfo info = RelTableCatalogInfo::deserialize(deSer); DASSERT(!tables.contains(info.oid)); - if (!relGroupEntry->getStorage().empty()) { + if (!relGroupEntry->getStorage().empty() && + TableOptionConstants::isIceBugDiskStorage(relGroupEntry->getStorage())) { // Create icebug-disk-backed rel table tables[info.oid] = std::make_unique(relGroupEntry, info.nodePair.srcTableID, info.nodePair.dstTableID, this, &memoryManager); diff --git a/test/test_files/ice_disk/ice_disk_invalid_storage.test b/test/test_files/ice_disk/ice_disk_invalid_storage.test index be0ea1e929..f255336c6f 100644 --- a/test/test_files/ice_disk/ice_disk_invalid_storage.test +++ b/test/test_files/ice_disk/ice_disk_invalid_storage.test @@ -4,31 +4,31 @@ -CASE IceDiskNonExistentDirNodeFails -LOG IceDiskNonExistentDirMissingParquet --STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:/nonexistent/dir/that/does/not/exist') +-STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (storage = 'icebug-disk:/nonexistent/dir/that/does/not/exist') ---- error(regex) .*[Nn]o such file.*|.*[Cc]annot open.*|.*nodes_t\.parquet.* -CASE IceDiskURINodeFails -LOG IceDiskURIMissingParquet --STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:https://example.com/path') +-STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (storage = 'icebug-disk:https://example.com/path') ---- error(regex) .*[Nn]o such file.*|.*[Cc]annot open.*|.*nodes_t\.parquet.* -CASE IceDiskPrefixOnlyRelFails -LOG IceDiskPrefixOnlyRelMissingParquet --STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') +-STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (storage = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') ---- ok --STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) WITH (STORAGE = 'icebug-disk') +-STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) WITH (storage = 'icebug-disk') ---- error(regex) .*[Nn]o such file.*|.*[Cc]annot open.*|.*indices_rel\.parquet.* -CASE IceDiskPrefixWithColonRelFails -LOG IceDiskPrefixColonRelMissingParquet --STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') +-STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (storage = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') ---- ok --STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) WITH (STORAGE = 'icebug-disk:') +-STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) WITH (storage = 'icebug-disk:') ---- error(regex) .*[Nn]o such file.*|.*[Cc]annot open.*|.*indices_rel\.parquet.* diff --git a/test/test_helper/test_helper.cpp b/test/test_helper/test_helper.cpp index 239c03fe1c..eb16e829a0 100644 --- a/test/test_helper/test_helper.cpp +++ b/test/test_helper/test_helper.cpp @@ -130,26 +130,6 @@ void TestHelper::executeScript(const std::string& cypherScript, Connection& conn } continue; } - - if (storagePath.find("://") != std::string::npos) { - // Non-icebug-disk URI — do not modify. - continue; - } - - auto fullPath = storagePath; - if (std::filesystem::path(storagePath).is_relative()) { - if (std::filesystem::path(storagePath).parent_path().empty()) { - fullPath = (cypherDir / storagePath).string(); - } else { - fullPath = appendLbugRootPath(storagePath); - } - } - fullPath = normalizePathForCypher(std::move(fullPath)); - - size_t pos = line.find(storagePath); - if (pos != std::string::npos) { - line.replace(pos, storagePath.length(), fullPath); - } } #ifdef __STATIC_LINK_EXTENSION_TEST__ if (line.starts_with("load extension")) { From daea73921e445d245210fcba34a869d7dabb0f94 Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Mon, 11 May 2026 21:36:19 +0530 Subject: [PATCH 04/10] update dataset submodule --- dataset | 2 +- test/storage/ice_disk_utils_test.cpp | 7 ------- test/test_files/ice_disk/ice_disk_mix_tables.test | 6 ++++-- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/dataset b/dataset index 55531118c5..30632c3f24 160000 --- a/dataset +++ b/dataset @@ -1 +1 @@ -Subproject commit 55531118c5e0c683fc3a3d806b7abd0b09a31ff8 +Subproject commit 30632c3f24cffe473c992ae8d29f278b654807a0 diff --git a/test/storage/ice_disk_utils_test.cpp b/test/storage/ice_disk_utils_test.cpp index f0290a515b..3887f1d373 100644 --- a/test/storage/ice_disk_utils_test.cpp +++ b/test/storage/ice_disk_utils_test.cpp @@ -21,9 +21,6 @@ static const std::string DEMO_DB_ICEBUG_DISK = // ───────────────────────────────────────────────────────────── // getBasePath // ───────────────────────────────────────────────────────────── -TEST(IceDiskUtils_GetBasePath, EmptyString) { - EXPECT_EQ("", IceDiskUtils::getBasePath("")); -} TEST(IceDiskUtils_GetBasePath, PrefixOnly) { EXPECT_EQ("", IceDiskUtils::getBasePath("icebug-disk")); @@ -41,10 +38,6 @@ TEST(IceDiskUtils_GetBasePath, PrefixWithRelativePath) { EXPECT_EQ("rel/path", IceDiskUtils::getBasePath("icebug-disk:rel/path")); } -TEST(IceDiskUtils_GetBasePath, NonIceDiskPrefix) { - EXPECT_EQ("", IceDiskUtils::getBasePath("parquet:/path")); -} - TEST(IceDiskUtils_GetBasePath, PrefixWithDot) { EXPECT_EQ(".", IceDiskUtils::getBasePath("icebug-disk:.")); } diff --git a/test/test_files/ice_disk/ice_disk_mix_tables.test b/test/test_files/ice_disk/ice_disk_mix_tables.test index 6c8a97997a..283eab68f9 100644 --- a/test/test_files/ice_disk/ice_disk_mix_tables.test +++ b/test/test_files/ice_disk/ice_disk_mix_tables.test @@ -18,14 +18,16 @@ Binder exception: icebug-disk rel tables require both FROM and TO tables to be i -STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') ---- ok --LOG RegularRelWithIceDiskNodeFails +-LOG CreateRegularNodeTable -STATEMENT CREATE NODE TABLE person(id INT64 PRIMARY KEY, name STRING) +---- ok + +-LOG RegularRelWithIceDiskNodeFails -STATEMENT CREATE REL TABLE rel(FROM person TO upperversion) ---- error Binder exception: Rel tables with non-icebug-disk storage do not support icebug-disk node tables as FROM or TO tables. -LOG RegularRelWithBothIceDiskNodeFails --STATEMENT CREATE REL TABLE rel(FROM person TO upperversion) -STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) ---- error Binder exception: Rel tables with non-icebug-disk storage do not support icebug-disk node tables as FROM or TO tables. From a839d188ae994f28c4d53e97b9b2d96e5eca22dc Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Mon, 11 May 2026 22:34:14 +0530 Subject: [PATCH 05/10] add support for lbug -i .//schema.cypher --- src/include/storage/storage_manager.h | 12 ++++---- .../storage/table/ice_disk_node_table.h | 6 +++- .../storage/table/ice_disk_rel_table.h | 5 +++- src/include/storage/table/ice_disk_utils.h | 24 +++++++++++++--- src/processor/operator/ddl/alter.cpp | 2 +- src/processor/operator/ddl/create_table.cpp | 3 +- src/storage/storage_manager.cpp | 28 +++++++++++-------- src/storage/table/ice_disk_node_table.cpp | 7 +++-- src/storage/table/ice_disk_rel_table.cpp | 12 ++++---- .../alter_table_entry_record_replay.cpp | 2 +- .../create_catalog_entry_record_replay.cpp | 2 +- 11 files changed, 67 insertions(+), 36 deletions(-) diff --git a/src/include/storage/storage_manager.h b/src/include/storage/storage_manager.h index 409708ccdd..8f95064338 100644 --- a/src/include/storage/storage_manager.h +++ b/src/include/storage/storage_manager.h @@ -44,9 +44,9 @@ class LBUG_API StorageManager { static void recover(main::ClientContext& clientContext, bool throwOnWalReplayFailure, bool enableChecksums); - void createTable(catalog::TableCatalogEntry* entry); - void addRelTable(catalog::RelGroupCatalogEntry* entry, - const catalog::RelTableCatalogInfo& info); + void createTable(catalog::TableCatalogEntry* entry, main::ClientContext* context = nullptr); + void addRelTable(catalog::RelGroupCatalogEntry* entry, const catalog::RelTableCatalogInfo& info, + main::ClientContext* context = nullptr); bool checkpoint(main::ClientContext* context, PageAllocator& pageAllocator); bool checkpoint(main::ClientContext* context, const transaction::Transaction& snapshotTxn, @@ -100,9 +100,11 @@ class LBUG_API StorageManager { static StorageManager* Get(const main::ClientContext& context); private: - void createNodeTable(catalog::NodeTableCatalogEntry* entry); + void createNodeTable(catalog::NodeTableCatalogEntry* entry, + main::ClientContext* context = nullptr); - void createRelTableGroup(catalog::RelGroupCatalogEntry* entry); + void createRelTableGroup(catalog::RelGroupCatalogEntry* entry, + main::ClientContext* context = nullptr); void reclaimDroppedTables(const catalog::Catalog& catalog); diff --git a/src/include/storage/table/ice_disk_node_table.h b/src/include/storage/table/ice_disk_node_table.h index eb64cfc478..f8d31ed23e 100644 --- a/src/include/storage/table/ice_disk_node_table.h +++ b/src/include/storage/table/ice_disk_node_table.h @@ -12,6 +12,9 @@ #include "storage/table/columnar_node_table_base.h" namespace lbug { +namespace main { +class ClientContext; +} // namespace main namespace storage { struct IceDiskNodeTableScanState final : ColumnarNodeTableScanState { @@ -61,7 +64,8 @@ struct IceDiskNodeTableScanSharedState final : ColumnarNodeTableScanSharedState class IceDiskNodeTable final : public ColumnarNodeTableBase { public: IceDiskNodeTable(const StorageManager* storageManager, - const catalog::NodeTableCatalogEntry* nodeTableEntry, MemoryManager* memoryManager); + const catalog::NodeTableCatalogEntry* nodeTableEntry, MemoryManager* memoryManager, + main::ClientContext* context = nullptr); void initializeScanCoordination(const transaction::Transaction* transaction) override; diff --git a/src/include/storage/table/ice_disk_rel_table.h b/src/include/storage/table/ice_disk_rel_table.h index ec7488c42a..975424d6db 100644 --- a/src/include/storage/table/ice_disk_rel_table.h +++ b/src/include/storage/table/ice_disk_rel_table.h @@ -8,6 +8,9 @@ #include "transaction/transaction.h" namespace lbug { +namespace main { +class ClientContext; +} // namespace main namespace storage { struct IceDiskRelTableScanState final : RelTableScanState { @@ -38,7 +41,7 @@ class IceDiskRelTable final : public ColumnarRelTableBase { public: IceDiskRelTable(catalog::RelGroupCatalogEntry* relGroupEntry, common::table_id_t fromTableID, common::table_id_t toTableID, const StorageManager* storageManager, - MemoryManager* memoryManager); + MemoryManager* memoryManager, main::ClientContext* context = nullptr); void initScanState(transaction::Transaction* transaction, TableScanState& scanState, bool resetCachedBoundNodeSelVec = true) const override; diff --git a/src/include/storage/table/ice_disk_utils.h b/src/include/storage/table/ice_disk_utils.h index 6fd2bd3fbd..bae4be3e1a 100644 --- a/src/include/storage/table/ice_disk_utils.h +++ b/src/include/storage/table/ice_disk_utils.h @@ -80,15 +80,30 @@ class IceDiskUtils { } // Validates that the parquet file at `path` carries the expected icebug_disk_version metadata. - // `dbDirectory` is used to anchor relative paths (typically parent of the .ladybug file). - static void checkVersionCompatibility(common::VirtualFileSystem* vfs, - const std::string& dbDirectory, const std::string& path) { + // `dbDirectory` is used to anchor relative paths (typically parent of the .lbdb file). + // When the resolved path does not exist and `context` is provided, falls back to VFS + // search-path resolution (fileSearchPath) so that `lbug -i schema.cypher` can find parquet + // files located in the same directory as the schema file. + // Returns the final resolved path that was successfully validated. + static std::string checkVersionCompatibility(common::VirtualFileSystem* vfs, + const std::string& dbDirectory, const std::string& path, + main::ClientContext* context = nullptr) { if (!vfs) { throw common::RuntimeException( "No VirtualFileSystem available for IceDisk version check: " + path); } - const auto resolvedPath = resolveIceDiskPath(path, dbDirectory); + auto resolvedPath = resolveIceDiskPath(path, dbDirectory); + + // When the primary (DB-relative) path does not exist and the caller provides a + // client context, try resolving through the VFS file-search path (e.g. the directory + // added by addInitFileDirToSearchPath when -i is used). + if (context && !vfs->fileOrPathExists(resolvedPath)) { + auto found = vfs->glob(context, path); + if (!found.empty()) { + resolvedPath = found.front(); + } + } auto metadata = processor::ParquetReader::readMetadata(resolvedPath, vfs); if (!metadata) { @@ -123,6 +138,7 @@ class IceDiskUtils { throw common::RuntimeException( "Current ladybug version does not support icebug_disk_version: " + versionValue); } + return resolvedPath; } }; diff --git a/src/processor/operator/ddl/alter.cpp b/src/processor/operator/ddl/alter.cpp index 6e99af8b70..72da92f3f3 100644 --- a/src/processor/operator/ddl/alter.cpp +++ b/src/processor/operator/ddl/alter.cpp @@ -346,7 +346,7 @@ void Alter::alterTable(main::ClientContext* clientContext, const TableCatalogEnt auto connectionInfo = alterInfo.extraInfo->constPtrCast(); auto relEntryInfo = relGroupEntry->getRelEntryInfo(connectionInfo->fromTableID, connectionInfo->toTableID); - storageManager->addRelTable(relGroupEntry, *relEntryInfo); + storageManager->addRelTable(relGroupEntry, *relEntryInfo, clientContext); } break; default: break; diff --git a/src/processor/operator/ddl/create_table.cpp b/src/processor/operator/ddl/create_table.cpp index d98e9ee40a..d3479c3b49 100644 --- a/src/processor/operator/ddl/create_table.cpp +++ b/src/processor/operator/ddl/create_table.cpp @@ -42,7 +42,8 @@ void CreateTable::executeInternal(ExecutionContext* context) { default: UNREACHABLE_CODE; } - storage::StorageManager::Get(*clientContext)->createTable(entry->ptrCast()); + storage::StorageManager::Get(*clientContext) + ->createTable(entry->ptrCast(), clientContext); appendMessage(std::format("Table {} has been created.", info.tableName), memoryManager); sharedState->tableCreated = true; } diff --git a/src/storage/storage_manager.cpp b/src/storage/storage_manager.cpp index 6e3d33d4b9..821d0d118e 100644 --- a/src/storage/storage_manager.cpp +++ b/src/storage/storage_manager.cpp @@ -97,7 +97,7 @@ void StorageManager::recover(main::ClientContext& clientContext, bool throwOnWal walReplayer->replay(throwOnWalReplayFailure, enableChecksums); } -void StorageManager::createNodeTable(NodeTableCatalogEntry* entry) { +void StorageManager::createNodeTable(NodeTableCatalogEntry* entry, main::ClientContext* context) { tableNameCache[entry->getTableID()] = entry->getName(); if (!entry->getStorage().empty()) { // Check if storage is Arrow backed @@ -126,7 +126,7 @@ void StorageManager::createNodeTable(NodeTableCatalogEntry* entry) { } else if (TableOptionConstants::isIceBugDiskStorage(entry->getStorage())) { // Create icebug-disk-backed node table tables[entry->getTableID()] = - std::make_unique(this, entry, &memoryManager); + std::make_unique(this, entry, &memoryManager, context); } else { throw common::RuntimeException( "Unsupported storage option for node table: " + entry->getStorage()); @@ -140,7 +140,8 @@ void StorageManager::createNodeTable(NodeTableCatalogEntry* entry) { // TODO(Guodong): This API is added since storageManager doesn't provide an API to add a single // rel table. We may have to refactor the existing StorageManager::createTable(TableCatalogEntry* // entry). -void StorageManager::addRelTable(RelGroupCatalogEntry* entry, const RelTableCatalogInfo& info) { +void StorageManager::addRelTable(RelGroupCatalogEntry* entry, const RelTableCatalogInfo& info, + main::ClientContext* context) { if (entry->getScanFunction().has_value()) { // Create foreign-backed rel table tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, @@ -177,7 +178,7 @@ void StorageManager::addRelTable(RelGroupCatalogEntry* entry, const RelTableCata } else if (TableOptionConstants::isIceBugDiskStorage(entry->getStorage())) { // Create icebug-disk-backed rel table tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, - info.nodePair.dstTableID, this, &memoryManager); + info.nodePair.dstTableID, this, &memoryManager, context); } else { throw common::RuntimeException( "Unsupported storage option for rel table: " + entry->getStorage()); @@ -189,20 +190,21 @@ void StorageManager::addRelTable(RelGroupCatalogEntry* entry, const RelTableCata } } -void StorageManager::createRelTableGroup(RelGroupCatalogEntry* entry) { +void StorageManager::createRelTableGroup(RelGroupCatalogEntry* entry, + main::ClientContext* context) { for (auto& info : entry->getRelEntryInfos()) { - addRelTable(entry, info); + addRelTable(entry, info, context); } } -void StorageManager::createTable(TableCatalogEntry* entry) { +void StorageManager::createTable(TableCatalogEntry* entry, main::ClientContext* context) { std::unique_lock lck{mtx}; switch (entry->getType()) { case CatalogEntryType::NODE_TABLE_ENTRY: { - createNodeTable(entry->ptrCast()); + createNodeTable(entry->ptrCast(), context); } break; case CatalogEntryType::REL_GROUP_ENTRY: { - createRelTableGroup(entry->ptrCast()); + createRelTableGroup(entry->ptrCast(), context); } break; default: { UNREACHABLE_CODE; @@ -446,7 +448,8 @@ void StorageManager::deserialize(main::ClientContext* context, const Catalog* ca tableNameCache[tableID] = tableEntry->getName(); if (!tableEntry->getStorage().empty()) { // Create icebug-disk-backed node table - tables[tableID] = std::make_unique(this, tableEntry, &memoryManager); + tables[tableID] = + std::make_unique(this, tableEntry, &memoryManager, context); } else { // Create regular node table tables[tableID] = std::make_unique(this, tableEntry, &memoryManager); @@ -475,8 +478,9 @@ void StorageManager::deserialize(main::ClientContext* context, const Catalog* ca if (!relGroupEntry->getStorage().empty() && TableOptionConstants::isIceBugDiskStorage(relGroupEntry->getStorage())) { // Create icebug-disk-backed rel table - tables[info.oid] = std::make_unique(relGroupEntry, - info.nodePair.srcTableID, info.nodePair.dstTableID, this, &memoryManager); + tables[info.oid] = + std::make_unique(relGroupEntry, info.nodePair.srcTableID, + info.nodePair.dstTableID, this, &memoryManager, context); } else { // Create regular rel table tables[info.oid] = std::make_unique(relGroupEntry, diff --git a/src/storage/table/ice_disk_node_table.cpp b/src/storage/table/ice_disk_node_table.cpp index be46195a3a..e1e35184c4 100644 --- a/src/storage/table/ice_disk_node_table.cpp +++ b/src/storage/table/ice_disk_node_table.cpp @@ -26,7 +26,8 @@ namespace lbug { namespace storage { IceDiskNodeTable::IceDiskNodeTable(const StorageManager* storageManager, - const NodeTableCatalogEntry* nodeTableEntry, MemoryManager* memoryManager) + const NodeTableCatalogEntry* nodeTableEntry, MemoryManager* memoryManager, + main::ClientContext* context) : ColumnarNodeTableBase{storageManager, nodeTableEntry, memoryManager, std::make_unique()} { auto file = IceDiskUtils::constructNodeTablePath( @@ -34,8 +35,8 @@ IceDiskNodeTable::IceDiskNodeTable(const StorageManager* storageManager, ".parquet"); const auto dbDir = std::filesystem::path(storageManager->getDatabasePath()).parent_path().string(); - IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, file); - parquetFilePath = file; + parquetFilePath = + IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, file, context); } void IceDiskNodeTable::initializeScanCoordination(const transaction::Transaction* transaction) { diff --git a/src/storage/table/ice_disk_rel_table.cpp b/src/storage/table/ice_disk_rel_table.cpp index 01f3324777..b452b700c7 100644 --- a/src/storage/table/ice_disk_rel_table.cpp +++ b/src/storage/table/ice_disk_rel_table.cpp @@ -43,18 +43,18 @@ void IceDiskRelTableScanState::setToTable(const Transaction* transaction, Table* } IceDiskRelTable::IceDiskRelTable(RelGroupCatalogEntry* relGroupEntry, table_id_t fromTableID, - table_id_t toTableID, const StorageManager* storageManager, MemoryManager* memoryManager) + table_id_t toTableID, const StorageManager* storageManager, MemoryManager* memoryManager, + main::ClientContext* context) : ColumnarRelTableBase{relGroupEntry, fromTableID, toTableID, storageManager, memoryManager} { const auto base = IceDiskUtils::getBasePath(relGroupEntry->getStorage()); auto paths = IceDiskUtils::constructCSRPaths(base, relGroupEntry->getName(), ".parquet"); const auto dbDir = std::filesystem::path(storageManager->getDatabasePath()).parent_path().string(); - IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, paths.indices); - IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, paths.indptr); - - indicesFilePath = paths.indices; - indptrFilePath = paths.indptr; + indicesFilePath = IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, + paths.indices, context); + indptrFilePath = IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, + paths.indptr, context); } void IceDiskRelTable::initScanState(Transaction* transaction, TableScanState& scanState, diff --git a/src/storage/wal/records/alter_table_entry_record_replay.cpp b/src/storage/wal/records/alter_table_entry_record_replay.cpp index f1c33c942e..ff90e2a294 100644 --- a/src/storage/wal/records/alter_table_entry_record_replay.cpp +++ b/src/storage/wal/records/alter_table_entry_record_replay.cpp @@ -61,7 +61,7 @@ void WALReplayer::replayAlterTableEntryRecord(const WALRecord& walRecord) const ->ptrCast(); auto relEntryInfo = relGroupEntry->getRelEntryInfo(extraInfo->fromTableID, extraInfo->toTableID); - storageManager->addRelTable(relGroupEntry, *relEntryInfo); + storageManager->addRelTable(relGroupEntry, *relEntryInfo, &clientContext); } break; default: break; diff --git a/src/storage/wal/records/create_catalog_entry_record_replay.cpp b/src/storage/wal/records/create_catalog_entry_record_replay.cpp index 2a9f002d14..b46e0b6245 100644 --- a/src/storage/wal/records/create_catalog_entry_record_replay.cpp +++ b/src/storage/wal/records/create_catalog_entry_record_replay.cpp @@ -24,7 +24,7 @@ void WALReplayer::replayCreateCatalogEntryRecord(WALRecord& walRecord) const { auto& entry = record.ownedCatalogEntry->constCast(); auto newEntry = catalog->createTableEntry(transaction, entry.getBoundCreateTableInfo(transaction, record.isInternal)); - storageManager->createTable(newEntry->ptrCast()); + storageManager->createTable(newEntry->ptrCast(), &clientContext); } break; case CatalogEntryType::SCALAR_MACRO_ENTRY: { auto& macroEntry = record.ownedCatalogEntry->constCast(); From 701d7226f2236f28f43b4fe6b6337cee81e94784 Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Tue, 12 May 2026 16:13:49 +0530 Subject: [PATCH 06/10] fix icebug-disk immutability on alter --- docs/icebug-disk.md | 2 + src/binder/bind/bind_ddl.cpp | 29 ++++++++++++++ .../storage/table/ice_disk_node_table.h | 2 +- .../demo_db/demo_db_icebug_disk.test | 40 +++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/docs/icebug-disk.md b/docs/icebug-disk.md index 98a925fcc3..bfc881c1b1 100644 --- a/docs/icebug-disk.md +++ b/docs/icebug-disk.md @@ -33,6 +33,8 @@ If the directory is moved, the affected tables must be dropped and re-created wi Mixed tables are not supported — `CREATE REL TABLE` queries involving both `icebug-disk` and non-`icebug-disk` tables will throw a `BinderException`. +Note: Icebug-disk tables are **immutable**: `ALTER TABLE` operations, Data insertions, updates, and deletions are not supported. + ### Node tables For each node table, there is a corresponding Parquet file named `nodes_{tableName}.parquet` containing a primary key column and one column per property as declared in the schema. diff --git a/src/binder/bind/bind_ddl.cpp b/src/binder/bind/bind_ddl.cpp index 26e3822c71..d1656356a2 100644 --- a/src/binder/bind/bind_ddl.cpp +++ b/src/binder/bind/bind_ddl.cpp @@ -10,6 +10,7 @@ #include "binder/expression_visitor.h" #include "catalog/catalog.h" #include "catalog/catalog_entry/node_table_catalog_entry.h" +#include "catalog/catalog_entry/rel_group_catalog_entry.h" #include "catalog/catalog_entry/sequence_catalog_entry.h" #include "common/constants.h" #include "common/enums/extend_direction_util.h" @@ -560,8 +561,36 @@ std::unique_ptr Binder::bindDrop(const Statement& statement) { return std::make_unique(drop.getDropInfo()); } +static void validateNotIceDiskTable(main::ClientContext* clientContext, + const std::string& tableName) { + auto catalog = Catalog::Get(*clientContext); + auto transaction = transaction::Transaction::Get(*clientContext); + + if (!catalog->containsTable(transaction, tableName)) { + return; + } + + auto tableEntry = catalog->getTableCatalogEntry(transaction, tableName); + std::string storage; + + if (tableEntry->getTableType() == common::TableType::NODE) { + storage = tableEntry->ptrCast()->getStorage(); + } else if (tableEntry->getTableType() == common::TableType::REL) { + storage = tableEntry->ptrCast()->getStorage(); + } + + if (TableOptionConstants::isIceBugDiskStorage(storage)) { + throw BinderException( + std::format("Cannot alter table {}: icebug-disk tables are immutable.", tableName)); + } +} + std::unique_ptr Binder::bindAlter(const Statement& statement) { auto& alter = statement.constCast(); + + // we don't support alter operations on icebug-disk tables + validateNotIceDiskTable(clientContext, alter.getInfo()->tableName); + switch (alter.getInfo()->type) { case AlterType::RENAME: { return bindRenameTable(statement); diff --git a/src/include/storage/table/ice_disk_node_table.h b/src/include/storage/table/ice_disk_node_table.h index f8d31ed23e..12a085c64d 100644 --- a/src/include/storage/table/ice_disk_node_table.h +++ b/src/include/storage/table/ice_disk_node_table.h @@ -83,7 +83,7 @@ class IceDiskNodeTable final : public ColumnarNodeTableBase { protected: // Implement ColumnarNodeTableBase interface - std::string getColumnarFormatName() const override { return "icebug-"; } + std::string getColumnarFormatName() const override { return "icebug-disk"; } common::node_group_idx_t getNumBatches( const transaction::Transaction* transaction) const override; common::row_idx_t getTotalRowCount(const transaction::Transaction* transaction) const override; diff --git a/test/test_files/demo_db/demo_db_icebug_disk.test b/test/test_files/demo_db/demo_db_icebug_disk.test index 5e3adfe00c..18462de648 100644 --- a/test/test_files/demo_db/demo_db_icebug_disk.test +++ b/test/test_files/demo_db/demo_db_icebug_disk.test @@ -4,6 +4,46 @@ -CASE DemoDBIcebugDiskTest +-LOG RenameUserNodeTableFails +-STATEMENT ALTER TABLE user RENAME TO user2 +---- error +Binder exception: Cannot alter table user: icebug-disk tables are immutable. + +-LOG RenameFollowsRelTableFails +-STATEMENT ALTER TABLE follows RENAME TO follows2 +---- error +Binder exception: Cannot alter table follows: icebug-disk tables are immutable. + +-LOG InsertIntoUserNodeTableFails +-STATEMENT CREATE (u:user {id: 350, name: 'Alice', age: 35}); +---- error(regex) +^Runtime exception: Cannot insert.* + +-LOG InsertIntoFollowsRelTableFails +-STATEMENT MATCH (u1:User), (u2:User) WHERE u1.name = 'Adam' AND u2.name = 'Noura' CREATE (u1)-[:Follows {since: 2011}]->(u2); +---- error(regex) +^Runtime exception: Cannot insert.* + +-LOG UpdateUserNodeFails +-STATEMENT MATCH (u:user {name: 'Adam'}) SET u.age = 31 +---- error(regex) +^Runtime exception: Cannot update.* + +-LOG UpdateFollowsRelFails +-STATEMENT MATCH (u1:User)-[f:Follows]->(u2:User) WHERE u1.name = 'Adam' AND u2.name = 'Karissa' SET f.since = 2011 +---- error(regex) +^Runtime exception: Cannot update.* + +-LOG DeleteUserNodeFails +-STATEMENT MATCH (u:user {name: 'Adam'}) DELETE u +---- error(regex) +^Runtime exception: Cannot delete.* + +-LOG DeleteFollowsRelFails +-STATEMENT MATCH (u1:User)-[f:Follows]->(u2:User) WHERE u1.name = 'Adam' AND u2.name = 'Karissa' DELETE f +---- error(regex) +^Runtime exception: Cannot delete.* + -LOG MatchUserLivesInCity -STATEMENT MATCH (u:user)-[l:livesin]->(c:city) RETURN u.name, u.age, c.name; ---- 4 From a243178c35113f817551081dcb2e615672784162 Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Tue, 12 May 2026 17:06:26 +0530 Subject: [PATCH 07/10] fix mix tables check --- src/binder/bind/bind_ddl.cpp | 36 ++++++++----------- .../ice_disk/ice_disk_mix_tables.test | 6 ++-- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/binder/bind/bind_ddl.cpp b/src/binder/bind/bind_ddl.cpp index d1656356a2..2ad3f56e3e 100644 --- a/src/binder/bind/bind_ddl.cpp +++ b/src/binder/bind/bind_ddl.cpp @@ -322,28 +322,22 @@ BoundCreateTableInfo Binder::bindCreateRelTableGroupInfo(const CreateTableInfo* } } + bool isSrcIcebugDisk = srcEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ? + TableOptionConstants::isIceBugDiskStorage( + srcEntry->ptrCast()->getStorage()) : + false; + bool isDstIcebugDisk = dstEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ? + TableOptionConstants::isIceBugDiskStorage( + dstEntry->ptrCast()->getStorage()) : + false; + bool isRelIcebugDisk = TableOptionConstants::isIceBugDiskStorage(storage); + + // 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 - if (TableOptionConstants::isIceBugDiskStorage(storage)) { - auto srcNodeEntry = srcEntry->ptrCast(); - auto dstNodeEntry = dstEntry->ptrCast(); - - if (!TableOptionConstants::isIceBugDiskStorage(srcNodeEntry->getStorage()) || - !TableOptionConstants::isIceBugDiskStorage(dstNodeEntry->getStorage())) { - throw BinderException("icebug-disk rel tables require both FROM and TO tables to " - "be icebug-disk node tables."); - } - } - - // Non-icebug-disk rel tables cannot connect to icebug-disk node tables - if (!TableOptionConstants::isIceBugDiskStorage(storage)) { - auto srcNodeEntry = srcEntry->ptrCast(); - auto dstNodeEntry = dstEntry->ptrCast(); - - if (TableOptionConstants::isIceBugDiskStorage(srcNodeEntry->getStorage()) || - TableOptionConstants::isIceBugDiskStorage(dstNodeEntry->getStorage())) { - throw BinderException("Rel tables with non-icebug-disk storage do not support " - "icebug-disk node tables as FROM or TO tables."); - } + if ((!isRelIcebugDisk && (isSrcIcebugDisk || isDstIcebugDisk)) || + (isRelIcebugDisk && (!isSrcIcebugDisk || !isDstIcebugDisk))) { + throw BinderException( + "Cannot mix icebug-disk tables with non-icebug-disk tables in CREATE REL TABLE."); } // Use the actual shadow table IDs, not FOREIGN_TABLE_ID diff --git a/test/test_files/ice_disk/ice_disk_mix_tables.test b/test/test_files/ice_disk/ice_disk_mix_tables.test index 283eab68f9..296bdfd5ae 100644 --- a/test/test_files/ice_disk/ice_disk_mix_tables.test +++ b/test/test_files/ice_disk/ice_disk_mix_tables.test @@ -10,7 +10,7 @@ -LOG IceDiskRelWithRegularNodeFails -STATEMENT CREATE REL TABLE rel(FROM person TO person) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') ---- error -Binder exception: icebug-disk rel tables require both FROM and TO tables to be icebug-disk node tables. +Binder exception: Cannot mix icebug-disk tables with non-icebug-disk tables in CREATE REL TABLE. -CASE NodeIceDiskRelRegular @@ -25,9 +25,9 @@ Binder exception: icebug-disk rel tables require both FROM and TO tables to be i -LOG RegularRelWithIceDiskNodeFails -STATEMENT CREATE REL TABLE rel(FROM person TO upperversion) ---- error -Binder exception: Rel tables with non-icebug-disk storage do not support icebug-disk node tables as FROM or TO tables. +Binder exception: Cannot mix icebug-disk tables with non-icebug-disk tables in CREATE REL TABLE. -LOG RegularRelWithBothIceDiskNodeFails -STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) ---- error -Binder exception: Rel tables with non-icebug-disk storage do not support icebug-disk node tables as FROM or TO tables. +Binder exception: Cannot mix icebug-disk tables with non-icebug-disk tables in CREATE REL TABLE. From d9be2b85aafb0374741d7d218755320b98dc60bf Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Tue, 12 May 2026 23:53:48 +0530 Subject: [PATCH 08/10] update ddl --- docs/icebug-disk.md | 12 ++- src/binder/bind/bind_ddl.cpp | 47 +++++---- src/catalog/catalog.cpp | 11 ++- .../node_table_catalog_entry.cpp | 23 ++++- .../catalog_entry/rel_group_catalog_entry.cpp | 21 +++- .../binder/ddl/bound_create_table_info.h | 21 ++-- .../catalog_entry/node_table_catalog_entry.h | 7 +- .../catalog_entry/rel_group_catalog_entry.h | 6 +- src/include/common/constants.h | 7 +- .../reader/parquet/parquet_reader.h | 5 - src/include/storage/table/ice_disk_utils.h | 64 +++--------- .../reader/parquet/parquet_reader.cpp | 37 ------- src/storage/storage_manager.cpp | 34 ++++--- src/storage/table/ice_disk_node_table.cpp | 21 ++-- src/storage/table/ice_disk_rel_table.cpp | 31 +++--- test/storage/ice_disk_utils_test.cpp | 99 ++++++++----------- .../ice_disk/ice_disk_invalid_storage.test | 26 +++-- .../ice_disk/ice_disk_mix_tables.test | 4 +- test/test_helper/test_helper.cpp | 83 ++++++++-------- 19 files changed, 265 insertions(+), 294 deletions(-) diff --git a/docs/icebug-disk.md b/docs/icebug-disk.md index bfc881c1b1..3f48fb6494 100644 --- a/docs/icebug-disk.md +++ b/docs/icebug-disk.md @@ -17,15 +17,17 @@ Version is stored in each file's metadata footer as a key-value pair: `icebug_di The graph schema is declared in a `schema.cypher`, which can be loaded using `lbug -i schema.cypher` to create tables in Ladybug. ```cypher -CREATE NODE TABLE city(id INT32, name STRING, population INT64, PRIMARY KEY(id)) WITH (storage = 'icebug-disk:'); -CREATE NODE TABLE user(id INT32, name STRING, age INT64, PRIMARY KEY(id)) WITH (storage = 'icebug-disk:'); -CREATE REL TABLE follows(FROM user TO user, since INT32) WITH (storage = 'icebug-disk:'); -CREATE REL TABLE livesin(FROM user TO city) WITH (storage = 'icebug-disk:'); +CREATE NODE TABLE city(id INT32, name STRING, population INT64, PRIMARY KEY(id)) WITH (storage = '', format = 'icebug-disk'); +CREATE NODE TABLE user(id INT32, name STRING, age INT64, PRIMARY KEY(id)) WITH (storage = '', format = 'icebug-disk'); +CREATE REL TABLE follows(FROM user TO user, since INT32) WITH (storage = '', format = 'icebug-disk'); +CREATE REL TABLE livesin(FROM user TO city) WITH (storage = '', format = 'icebug-disk'); ``` File paths can be relative or absolute and are resolved as `/nodes_{tableName}.parquet` for node tables, and `/indices_{tableName}.parquet` and `/indptr_{tableName}.parquet` for relationship tables. -`storage = 'icebug-disk'` and `storage = 'icebug-disk:'` both resolve to the current working directory. +Object-store URIs (e.g. `s3://bucket/path`, `https://host/path`) are supported as `storage` values and are passed through unchanged. + +If `storage` is omitted when `format = 'icebug-disk'` is set, files are resolved relative to the current working directory. Tables can also be created by manually running the above queries in the Ladybug CLI. diff --git a/src/binder/bind/bind_ddl.cpp b/src/binder/bind/bind_ddl.cpp index 2ad3f56e3e..fe2acbdd5d 100644 --- a/src/binder/bind/bind_ddl.cpp +++ b/src/binder/bind/bind_ddl.cpp @@ -192,14 +192,23 @@ static ExtendDirection getStorageDirection(const case_insensitive_map_t& return DEFAULT_EXTEND_DIRECTION; } +static std::string getStorageFormat(const case_insensitive_map_t& options) { + if (options.contains(TableOptionConstants::STORAGE_FORMAT_OPTION)) { + return options.at(TableOptionConstants::STORAGE_FORMAT_OPTION).toString(); + } + + return ""; +} + BoundCreateTableInfo Binder::bindCreateNodeTableInfo(const CreateTableInfo* info) { auto propertyDefinitions = bindPropertyDefinitions(info->propertyDefinitions, info->tableName); auto& extraInfo = info->extraInfo->constCast(); validatePrimaryKey(extraInfo.pKName, propertyDefinitions); auto boundOptions = bindParsingOptions(extraInfo.options); auto storage = getStorage(boundOptions); + auto storageFormat = getStorageFormat(boundOptions); auto boundExtraInfo = std::make_unique(extraInfo.pKName, - std::move(propertyDefinitions), std::move(storage)); + std::move(propertyDefinitions), std::move(storage), std::move(storageFormat)); return BoundCreateTableInfo(CatalogEntryType::NODE_TABLE_ENTRY, info->tableName, info->onConflict, std::move(boundExtraInfo), clientContext->useInternalCatalogEntry()); } @@ -222,6 +231,7 @@ BoundCreateTableInfo Binder::bindCreateRelTableGroupInfo(const CreateTableInfo* auto boundOptions = bindParsingOptions(extraInfo.options); auto storageDirection = getStorageDirection(boundOptions); auto storage = getStorage(boundOptions); + auto storageFormat = getStorageFormat(boundOptions); std::optional scanFunction = std::nullopt; std::optional> scanBindData = std::nullopt; std::string foreignDatabaseName; @@ -231,7 +241,8 @@ 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::isIceBugDiskStorage(storage) && dotPos != std::string::npos) { + if (!TableOptionConstants::isIceBugDiskFormat(storageFormat) && + dotPos != std::string::npos) { std::string dbName = storage.substr(0, dotPos); std::string tableName = storage.substr(dotPos + 1); if (!dbName.empty()) { @@ -322,15 +333,17 @@ BoundCreateTableInfo Binder::bindCreateRelTableGroupInfo(const CreateTableInfo* } } - bool isSrcIcebugDisk = srcEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ? - TableOptionConstants::isIceBugDiskStorage( - srcEntry->ptrCast()->getStorage()) : - false; - bool isDstIcebugDisk = dstEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ? - TableOptionConstants::isIceBugDiskStorage( - dstEntry->ptrCast()->getStorage()) : - false; - bool isRelIcebugDisk = TableOptionConstants::isIceBugDiskStorage(storage); + bool isSrcIcebugDisk = + srcEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ? + TableOptionConstants::isIceBugDiskFormat( + srcEntry->ptrCast()->getStorageFormat()) : + false; + bool isDstIcebugDisk = + dstEntry->getType() == CatalogEntryType::NODE_TABLE_ENTRY ? + TableOptionConstants::isIceBugDiskFormat( + dstEntry->ptrCast()->getStorageFormat()) : + false; + bool isRelIcebugDisk = TableOptionConstants::isIceBugDiskFormat(storageFormat); // 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 @@ -354,8 +367,8 @@ BoundCreateTableInfo Binder::bindCreateRelTableGroupInfo(const CreateTableInfo* } auto boundExtraInfo = std::make_unique( std::move(propertyDefinitions), srcMultiplicity, dstMultiplicity, storageDirection, - std::move(nodePairs), std::move(storage), std::move(scanFunction), std::move(scanBindData), - std::move(foreignDatabaseName)); + std::move(nodePairs), std::move(storage), std::move(storageFormat), std::move(scanFunction), + std::move(scanBindData), std::move(foreignDatabaseName)); return BoundCreateTableInfo(CatalogEntryType::REL_GROUP_ENTRY, info->tableName, info->onConflict, std::move(boundExtraInfo), clientContext->useInternalCatalogEntry()); } @@ -565,15 +578,15 @@ static void validateNotIceDiskTable(main::ClientContext* clientContext, } auto tableEntry = catalog->getTableCatalogEntry(transaction, tableName); - std::string storage; + std::string storageFormat; if (tableEntry->getTableType() == common::TableType::NODE) { - storage = tableEntry->ptrCast()->getStorage(); + storageFormat = tableEntry->ptrCast()->getStorageFormat(); } else if (tableEntry->getTableType() == common::TableType::REL) { - storage = tableEntry->ptrCast()->getStorage(); + storageFormat = tableEntry->ptrCast()->getStorageFormat(); } - if (TableOptionConstants::isIceBugDiskStorage(storage)) { + if (TableOptionConstants::isIceBugDiskFormat(storageFormat)) { throw BinderException( std::format("Cannot alter table {}: icebug-disk tables are immutable.", tableName)); } diff --git a/src/catalog/catalog.cpp b/src/catalog/catalog.cpp index 930dbc2e68..a565b7285c 100644 --- a/src/catalog/catalog.cpp +++ b/src/catalog/catalog.cpp @@ -203,10 +203,11 @@ CatalogEntry* Catalog::createRelGroupEntry(Transaction* transaction, for (auto& nodePair : extraInfo->nodePairs) { relTableInfos.emplace_back(nodePair, tables->getNextOID()); } - auto relGroupEntry = std::make_unique(info.tableName, - extraInfo->srcMultiplicity, extraInfo->dstMultiplicity, extraInfo->storageDirection, - std::move(relTableInfos), extraInfo->storage, extraInfo->scanFunction, - std::move(extraInfo->scanBindData), extraInfo->foreignDatabaseName); + auto relGroupEntry = + std::make_unique(info.tableName, extraInfo->srcMultiplicity, + extraInfo->dstMultiplicity, extraInfo->storageDirection, std::move(relTableInfos), + extraInfo->storage, extraInfo->storageFormat, extraInfo->scanFunction, + std::move(extraInfo->scanBindData), extraInfo->foreignDatabaseName); for (auto& definition : extraInfo->propertyDefinitions) { relGroupEntry->addProperty(definition); } @@ -561,7 +562,7 @@ CatalogEntry* Catalog::createNodeTableEntry(Transaction* transaction, const BoundCreateTableInfo& info) { const auto extraInfo = info.extraInfo->constPtrCast(); auto entry = std::make_unique(info.tableName, extraInfo->primaryKeyName, - extraInfo->storage); + extraInfo->storage, extraInfo->storageFormat); for (auto& definition : extraInfo->propertyDefinitions) { entry->addProperty(definition); } diff --git a/src/catalog/catalog_entry/node_table_catalog_entry.cpp b/src/catalog/catalog_entry/node_table_catalog_entry.cpp index ee7170fc13..588e332852 100644 --- a/src/catalog/catalog_entry/node_table_catalog_entry.cpp +++ b/src/catalog/catalog_entry/node_table_catalog_entry.cpp @@ -24,6 +24,8 @@ void NodeTableCatalogEntry::serialize(common::Serializer& serializer) const { serializer.write(primaryKeyName); serializer.writeDebuggingInfo("storage"); serializer.write(storage); + serializer.writeDebuggingInfo("storageFormat"); + serializer.write(storageFormat); } std::unique_ptr NodeTableCatalogEntry::deserialize( @@ -31,19 +33,35 @@ std::unique_ptr NodeTableCatalogEntry::deserialize( std::string debuggingInfo; std::string primaryKeyName; std::string storage; + std::string storageFormat; deserializer.validateDebuggingInfo(debuggingInfo, "primaryKeyName"); deserializer.deserializeValue(primaryKeyName); deserializer.validateDebuggingInfo(debuggingInfo, "storage"); deserializer.deserializeValue(storage); + deserializer.validateDebuggingInfo(debuggingInfo, "storageFormat"); + deserializer.deserializeValue(storageFormat); auto nodeTableEntry = std::make_unique(); nodeTableEntry->primaryKeyName = primaryKeyName; nodeTableEntry->storage = storage; + nodeTableEntry->storageFormat = storageFormat; return nodeTableEntry; } std::string NodeTableCatalogEntry::toCypher(const ToCypherInfo& /*info*/) const { - return std::format("CREATE NODE TABLE `{}` ({} PRIMARY KEY(`{}`));", getName(), + std::stringstream ss; + ss << std::format("CREATE NODE TABLE `{}` ({} PRIMARY KEY(`{}`))", getName(), propertyCollection.toCypher(), primaryKeyName); + + if (!storage.empty()) { + ss << std::format(" WITH (STORAGE = '{}'", storage); + if (!storageFormat.empty()) { + ss << std::format(", FORMAT = '{}'", storageFormat); + } + ss << ")"; + } + + ss << ";"; + return ss.str(); } std::optional NodeTableCatalogEntry::getScanFunction() const { @@ -66,6 +84,7 @@ std::unique_ptr NodeTableCatalogEntry::copy() const { auto other = std::make_unique(); other->primaryKeyName = primaryKeyName; other->storage = storage; + other->storageFormat = storageFormat; other->scanFunction = scanFunction; other->createBindDataFunc = createBindDataFunc; other->foreignDatabaseName = foreignDatabaseName; @@ -76,7 +95,7 @@ std::unique_ptr NodeTableCatalogEntry::copy() const { std::unique_ptr NodeTableCatalogEntry::getBoundExtraCreateInfo( transaction::Transaction*) const { return std::make_unique(primaryKeyName, - copyVector(getProperties()), storage); + copyVector(getProperties()), storage, storageFormat); } } // namespace catalog diff --git a/src/catalog/catalog_entry/rel_group_catalog_entry.cpp b/src/catalog/catalog_entry/rel_group_catalog_entry.cpp index 4b9f145bf2..8e637d6f46 100644 --- a/src/catalog/catalog_entry/rel_group_catalog_entry.cpp +++ b/src/catalog/catalog_entry/rel_group_catalog_entry.cpp @@ -105,6 +105,8 @@ void RelGroupCatalogEntry::serialize(Serializer& serializer) const { } serializer.writeDebuggingInfo("relTableInfos"); serializer.serializeVector(relTableInfos); + serializer.writeDebuggingInfo("storageFormat"); + serializer.serializeValue(storageFormat); } std::unique_ptr RelGroupCatalogEntry::deserialize( @@ -114,6 +116,7 @@ std::unique_ptr RelGroupCatalogEntry::deserialize( auto dstMultiplicity = RelMultiplicity::MANY; auto storageDirection = ExtendDirection::BOTH; std::string storage; + std::string storageFormat; std::vector relTableInfos; deserializer.validateDebuggingInfo(debuggingInfo, "srcMultiplicity"); deserializer.deserializeValue(srcMultiplicity); @@ -132,11 +135,14 @@ std::unique_ptr RelGroupCatalogEntry::deserialize( } deserializer.validateDebuggingInfo(debuggingInfo, "relTableInfos"); deserializer.deserializeVector(relTableInfos); + deserializer.validateDebuggingInfo(debuggingInfo, "storageFormat"); + deserializer.deserializeValue(storageFormat); auto relGroupEntry = std::make_unique(); relGroupEntry->srcMultiplicity = srcMultiplicity; relGroupEntry->dstMultiplicity = dstMultiplicity; relGroupEntry->storageDirection = storageDirection; relGroupEntry->storage = storage; + relGroupEntry->storageFormat = storageFormat; relGroupEntry->scanFunction = scanFunction; relGroupEntry->relTableInfos = relTableInfos; return relGroupEntry; @@ -171,7 +177,17 @@ std::string RelGroupCatalogEntry::toCypher(const ToCypherInfo& info) const { getFromToStr(relTableInfos[i].nodePair, catalog, transaction, storage)); } ss << ", " << propertyCollection.toCypher() << RelMultiplicityUtils::toString(srcMultiplicity) - << "_" << RelMultiplicityUtils::toString(dstMultiplicity) << ");"; + << "_" << RelMultiplicityUtils::toString(dstMultiplicity) << ")"; + + if (!storage.empty()) { + ss << std::format(" WITH (STORAGE = '{}'", storage); + if (!storageFormat.empty()) { + ss << std::format(", FORMAT = '{}'", storageFormat); + } + ss << ")"; + } + ss << ";"; + return ss.str(); } @@ -198,6 +214,7 @@ std::unique_ptr RelGroupCatalogEntry::copy() const { other->dstMultiplicity = dstMultiplicity; other->storageDirection = storageDirection; other->storage = storage; + other->storageFormat = storageFormat; other->scanFunction = scanFunction; other->scanBindData = std::nullopt; // TODO: implement copy for bindData if needed other->foreignDatabaseName = foreignDatabaseName; @@ -214,7 +231,7 @@ RelGroupCatalogEntry::getBoundExtraCreateInfo(transaction::Transaction*) const { } return std::make_unique( copyVector(propertyCollection.getDefinitions()), srcMultiplicity, dstMultiplicity, - storageDirection, std::move(nodePairs), storage); + storageDirection, std::move(nodePairs), storage, storageFormat); } } // namespace catalog diff --git a/src/include/binder/ddl/bound_create_table_info.h b/src/include/binder/ddl/bound_create_table_info.h index 2db5ac5d39..2466a0dead 100644 --- a/src/include/binder/ddl/bound_create_table_info.h +++ b/src/include/binder/ddl/bound_create_table_info.h @@ -76,14 +76,18 @@ struct LBUG_API BoundExtraCreateTableInfo : BoundExtraCreateCatalogEntryInfo { struct BoundExtraCreateNodeTableInfo final : BoundExtraCreateTableInfo { std::string primaryKeyName; std::string storage; + std::string storageFormat; BoundExtraCreateNodeTableInfo(std::string primaryKeyName, - std::vector definitions, std::string storage = "") + std::vector definitions, std::string storage = "", + std::string storageFormat = "") : BoundExtraCreateTableInfo{std::move(definitions)}, - primaryKeyName{std::move(primaryKeyName)}, storage{std::move(storage)} {} + primaryKeyName{std::move(primaryKeyName)}, storage{std::move(storage)}, + storageFormat{std::move(storageFormat)} {} BoundExtraCreateNodeTableInfo(const BoundExtraCreateNodeTableInfo& other) : BoundExtraCreateTableInfo{copyVector(other.propertyDefinitions)}, - primaryKeyName{other.primaryKeyName}, storage{other.storage} {} + primaryKeyName{other.primaryKeyName}, storage{other.storage}, + storageFormat{other.storageFormat} {} std::unique_ptr copy() const override { return std::make_unique(*this); @@ -96,6 +100,7 @@ struct BoundExtraCreateRelTableGroupInfo final : BoundExtraCreateTableInfo { common::ExtendDirection storageDirection; std::vector nodePairs; std::string storage; + std::string storageFormat; std::optional scanFunction; std::optional> scanBindData; std::string foreignDatabaseName; @@ -103,22 +108,24 @@ struct BoundExtraCreateRelTableGroupInfo final : BoundExtraCreateTableInfo { explicit BoundExtraCreateRelTableGroupInfo(std::vector definitions, common::RelMultiplicity srcMultiplicity, common::RelMultiplicity dstMultiplicity, common::ExtendDirection storageDirection, std::vector nodePairs, - std::string storage = "", + std::string storage = "", std::string storageFormat = "", std::optional scanFunction = std::nullopt, std::optional> scanBindData = std::nullopt, std::string foreignDatabaseName = "") : BoundExtraCreateTableInfo{std::move(definitions)}, srcMultiplicity{srcMultiplicity}, dstMultiplicity{dstMultiplicity}, storageDirection{storageDirection}, nodePairs{std::move(nodePairs)}, storage{std::move(storage)}, - scanFunction{std::move(scanFunction)}, scanBindData{std::move(scanBindData)}, + storageFormat{std::move(storageFormat)}, scanFunction{std::move(scanFunction)}, + scanBindData{std::move(scanBindData)}, foreignDatabaseName{std::move(foreignDatabaseName)} {} BoundExtraCreateRelTableGroupInfo(const BoundExtraCreateRelTableGroupInfo& other) : BoundExtraCreateTableInfo{copyVector(other.propertyDefinitions)}, srcMultiplicity{other.srcMultiplicity}, dstMultiplicity{other.dstMultiplicity}, storageDirection{other.storageDirection}, nodePairs{other.nodePairs}, - storage{other.storage}, scanFunction{other.scanFunction}, - scanBindData{other.scanBindData}, foreignDatabaseName{other.foreignDatabaseName} {} + storage{other.storage}, storageFormat{other.storageFormat}, + scanFunction{other.scanFunction}, scanBindData{other.scanBindData}, + foreignDatabaseName{other.foreignDatabaseName} {} std::unique_ptr copy() const override { return std::make_unique(*this); diff --git a/src/include/catalog/catalog_entry/node_table_catalog_entry.h b/src/include/catalog/catalog_entry/node_table_catalog_entry.h index 32dfd20a82..fe466c222a 100644 --- a/src/include/catalog/catalog_entry/node_table_catalog_entry.h +++ b/src/include/catalog/catalog_entry/node_table_catalog_entry.h @@ -27,9 +27,10 @@ class LBUG_API NodeTableCatalogEntry final : public TableCatalogEntry { public: NodeTableCatalogEntry() = default; - NodeTableCatalogEntry(std::string name, std::string primaryKeyName, std::string storage = "") + NodeTableCatalogEntry(std::string name, std::string primaryKeyName, std::string storage = "", + std::string storageFormat = "") : TableCatalogEntry{entryType_, std::move(name)}, primaryKeyName{std::move(primaryKeyName)}, - storage{std::move(storage)} {} + storage{std::move(storage)}, storageFormat{std::move(storageFormat)} {} // Constructor for foreign-backed tables NodeTableCatalogEntry(std::string name, std::string primaryKeyName, @@ -56,6 +57,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; } std::optional getScanFunction() const override; const CreateBindDataFunc& getCreateBindDataFunc() const { return createBindDataFunc; } const std::string& getForeignDatabaseName() const { return foreignDatabaseName; } @@ -82,6 +84,7 @@ class LBUG_API NodeTableCatalogEntry final : public TableCatalogEntry { private: std::string primaryKeyName; std::string storage; + std::string storageFormat; std::optional scanFunction; CreateBindDataFunc createBindDataFunc; // Callback to create bind data std::string foreignDatabaseName; diff --git a/src/include/catalog/catalog_entry/rel_group_catalog_entry.h b/src/include/catalog/catalog_entry/rel_group_catalog_entry.h index 807dc8f002..7d6e792bf1 100644 --- a/src/include/catalog/catalog_entry/rel_group_catalog_entry.h +++ b/src/include/catalog/catalog_entry/rel_group_catalog_entry.h @@ -39,13 +39,15 @@ class LBUG_API RelGroupCatalogEntry final : public TableCatalogEntry { RelGroupCatalogEntry(std::string tableName, common::RelMultiplicity srcMultiplicity, common::RelMultiplicity dstMultiplicity, common::ExtendDirection storageDirection, std::vector relTableInfos, std::string storage = "", + std::string storageFormat = "", std::optional scanFunction = std::nullopt, std::optional> scanBindData = std::nullopt, std::string foreignDatabaseName = "") : TableCatalogEntry{type_, std::move(tableName)}, srcMultiplicity{srcMultiplicity}, dstMultiplicity{dstMultiplicity}, storageDirection{storageDirection}, relTableInfos{std::move(relTableInfos)}, storage{std::move(storage)}, - scanFunction{std::move(scanFunction)}, scanBindData{std::move(scanBindData)}, + storageFormat{std::move(storageFormat)}, scanFunction{std::move(scanFunction)}, + scanBindData{std::move(scanBindData)}, foreignDatabaseName{std::move(foreignDatabaseName)} { propertyCollection = PropertyDefinitionCollection{1}; // Skip NBR_NODE_ID column as the first one. @@ -63,6 +65,7 @@ class LBUG_API RelGroupCatalogEntry final : public TableCatalogEntry { common::ExtendDirection getStorageDirection() const { return storageDirection; } const std::string& getStorage() const { return storage; } + const std::string& getStorageFormat() const { return storageFormat; } std::optional getScanFunction() const override { return scanFunction; } const std::optional>& getScanBindData() const { return scanBindData; @@ -113,6 +116,7 @@ class LBUG_API RelGroupCatalogEntry final : public TableCatalogEntry { common::ExtendDirection storageDirection = common::ExtendDirection::BOTH; std::vector relTableInfos; std::string storage; + std::string storageFormat; std::optional scanFunction; std::optional> scanBindData; std::string foreignDatabaseName; // Database name for foreign-backed rel tables diff --git a/src/include/common/constants.h b/src/include/common/constants.h index 536ee2176c..bf4c38ba1d 100644 --- a/src/include/common/constants.h +++ b/src/include/common/constants.h @@ -86,11 +86,12 @@ struct StorageConstants { struct TableOptionConstants { static constexpr char REL_STORAGE_DIRECTION_OPTION[] = "STORAGE_DIRECTION"; static constexpr char REL_STORAGE_OPTION[] = "STORAGE"; + static constexpr char STORAGE_FORMAT_OPTION[] = "FORMAT"; - static constexpr std::string_view ICEBUG_DISK_PREFIX = "icebug-disk"; + static constexpr std::string_view ICEBUG_DISK_FORMAT = "icebug-disk"; - static bool isIceBugDiskStorage(const std::string& storage) { - return storage.starts_with(ICEBUG_DISK_PREFIX); + static bool isIceBugDiskFormat(const std::string& format) { + return format.find(ICEBUG_DISK_FORMAT) != std::string::npos; } }; diff --git a/src/include/processor/operator/persistent/reader/parquet/parquet_reader.h b/src/include/processor/operator/persistent/reader/parquet/parquet_reader.h index ea89c1d516..c0a2cedd5f 100644 --- a/src/include/processor/operator/persistent/reader/parquet/parquet_reader.h +++ b/src/include/processor/operator/persistent/reader/parquet/parquet_reader.h @@ -56,11 +56,6 @@ class ParquetReader { lbug_parquet::format::FileMetaData* getMetadata() const { return metadata.get(); } - // Read only the parquet footer metadata from a file without requiring a ClientContext. - // Used for version checks at table construction time when no context is available. - static std::unique_ptr readMetadata( - const std::string& filePath, common::VirtualFileSystem* vfs); - private: std::unique_ptr createThriftProtocol( common::FileInfo* fileInfo_, bool prefetch_mode) { diff --git a/src/include/storage/table/ice_disk_utils.h b/src/include/storage/table/ice_disk_utils.h index bae4be3e1a..07b5942036 100644 --- a/src/include/storage/table/ice_disk_utils.h +++ b/src/include/storage/table/ice_disk_utils.h @@ -23,18 +23,6 @@ struct CSRFilePaths { class IceDiskUtils { public: - // Parses "icebug-disk", "icebug-disk:", or "icebug-disk:" and returns the path - // component. Returns empty string for the first two forms (caller interprets as current dir) - static std::string getBasePath(const std::string& storage) { - std::string_view rest = std::string_view(storage).substr( - common::TableOptionConstants::ICEBUG_DISK_PREFIX.size()); - // Strip the optional ':' separator. - if (!rest.empty() && rest[0] == ':') { - rest = rest.substr(1); - } - return std::string(rest); // empty means "current directory" - } - // Joins a base path with a filename. When base is empty the filename is returned // as-is (i.e. relative to the current working directory) static std::string joinPath(const std::string& base, const std::string& part) { @@ -62,50 +50,22 @@ class IceDiskUtils { IceDiskUtils::joinPath(dir, "indptr_" + name + suffix)}; } - // Resolves a potentially relative or ~-prefixed path against dbDirectory. - static std::string resolveIceDiskPath(const std::string& path, const std::string& dbDirectory) { - std::string expanded = path; - if (!expanded.empty() && expanded[0] == '~') { - const char* home = std::getenv("HOME"); - if (!home) { - throw common::RuntimeException( - "Cannot expand '~' in IceDisk path without HOME set: " + path); - } - expanded = std::string(home) + expanded.substr(1); - } - if (!std::filesystem::path(expanded).is_absolute()) { - expanded = (std::filesystem::path(dbDirectory) / expanded).lexically_normal().string(); - } - return expanded; - } - // Validates that the parquet file at `path` carries the expected icebug_disk_version metadata. - // `dbDirectory` is used to anchor relative paths (typically parent of the .lbdb file). - // When the resolved path does not exist and `context` is provided, falls back to VFS - // search-path resolution (fileSearchPath) so that `lbug -i schema.cypher` can find parquet - // files located in the same directory as the schema file. - // Returns the final resolved path that was successfully validated. - static std::string checkVersionCompatibility(common::VirtualFileSystem* vfs, - const std::string& dbDirectory, const std::string& path, - main::ClientContext* context = nullptr) { - if (!vfs) { + // Note: path is already resolved by VFS + static void checkVersionCompatibility(main::ClientContext* context, const std::string& path) { + if (!context) { throw common::RuntimeException( - "No VirtualFileSystem available for IceDisk version check: " + path); + path + ": failed to read parquet metadata for version check"); } - auto resolvedPath = resolveIceDiskPath(path, dbDirectory); - - // When the primary (DB-relative) path does not exist and the caller provides a - // client context, try resolving through the VFS file-search path (e.g. the directory - // added by addInitFileDirToSearchPath when -i is used). - if (context && !vfs->fileOrPathExists(resolvedPath)) { - auto found = vfs->glob(context, path); - if (!found.empty()) { - resolvedPath = found.front(); - } + auto tempReader = + std::make_unique(path, std::vector{}, context); + if (!tempReader) { + throw common::RuntimeException( + path + ": failed to read parquet metadata for version check"); } - auto metadata = processor::ParquetReader::readMetadata(resolvedPath, vfs); + auto metadata = tempReader->getMetadata(); if (!metadata) { throw common::RuntimeException( path + ": failed to read parquet metadata for version check"); @@ -136,9 +96,9 @@ class IceDiskUtils { if (!versionMatches) { throw common::RuntimeException( - "Current ladybug version does not support icebug_disk_version: " + versionValue); + path + + ": current ladybug version does not support icebug_disk_version: " + versionValue); } - return resolvedPath; } }; diff --git a/src/processor/operator/persistent/reader/parquet/parquet_reader.cpp b/src/processor/operator/persistent/reader/parquet/parquet_reader.cpp index 2879a37d36..0972426319 100644 --- a/src/processor/operator/persistent/reader/parquet/parquet_reader.cpp +++ b/src/processor/operator/persistent/reader/parquet/parquet_reader.cpp @@ -219,43 +219,6 @@ void ParquetReader::initMetadata() { metadata->read(proto.get()); } -std::unique_ptr ParquetReader::readMetadata(const std::string& filePath, - VirtualFileSystem* vfs) { - auto fileInfo = vfs->openFile(filePath, FileOpenFlags(FileFlags::READ_ONLY), nullptr); - auto proto = - std::make_unique>( - std::make_shared(fileInfo.get(), false)); - auto& transport = dynamic_cast_checked(*proto->getTransport()); - auto fileSize = transport.GetSize(); - if (fileSize < 12) { - throw CopyException{ - std::format("File {} is too small to be a Parquet file", filePath.c_str())}; - } - - ResizeableBuffer buf; - buf.resize(8); - buf.zero(); - - transport.SetLocation(fileSize - 8); - transport.read((uint8_t*)buf.ptr, 8); - - if (memcmp(buf.ptr + 4, "PAR1", 4) != 0) { - throw CopyException{ - std::format("No magic bytes found at the end of file {}", filePath.c_str())}; - } - auto footerLen = *reinterpret_cast(buf.ptr); - if (footerLen == 0 || fileSize < 12 + footerLen) { - throw CopyException{std::format("Footer length error in file {}", filePath.c_str())}; - } - auto metadataPos = fileSize - (footerLen + 8); - transport.SetLocation(metadataPos); - transport.Prefetch(metadataPos, footerLen); - - auto result = std::make_unique(); - result->read(proto.get()); - return result; -} - std::unique_ptr ParquetReader::createReaderRecursive(uint64_t depth, uint64_t maxDefine, uint64_t maxRepeat, uint64_t& nextSchemaIdx, uint64_t& nextFileIdx) { DASSERT(nextSchemaIdx < metadata->schema.size()); diff --git a/src/storage/storage_manager.cpp b/src/storage/storage_manager.cpp index 821d0d118e..6a986fbd75 100644 --- a/src/storage/storage_manager.cpp +++ b/src/storage/storage_manager.cpp @@ -99,7 +99,17 @@ void StorageManager::recover(main::ClientContext& clientContext, bool throwOnWal void StorageManager::createNodeTable(NodeTableCatalogEntry* entry, main::ClientContext* context) { tableNameCache[entry->getTableID()] = entry->getName(); - if (!entry->getStorage().empty()) { + + if (!entry->getStorageFormat().empty()) { + if (TableOptionConstants::isIceBugDiskFormat(entry->getStorageFormat())) { + // Create icebug-disk-backed node table + tables[entry->getTableID()] = + std::make_unique(this, entry, &memoryManager, context); + } else { + throw common::RuntimeException( + "Unsupported storage format option for node table: " + entry->getStorageFormat()); + } + } else if (!entry->getStorage().empty()) { // Check if storage is Arrow backed if (entry->getStorage().substr(0, 8) == "arrow://") { // Extract Arrow ID from storage string @@ -123,10 +133,6 @@ void StorageManager::createNodeTable(NodeTableCatalogEntry* entry, main::ClientC // Create Arrow-backed node table tables[entry->getTableID()] = std::make_unique(this, entry, &memoryManager, std::move(schemaCopy), std::move(arraysCopy), arrowId); - } else if (TableOptionConstants::isIceBugDiskStorage(entry->getStorage())) { - // Create icebug-disk-backed node table - tables[entry->getTableID()] = - std::make_unique(this, entry, &memoryManager, context); } else { throw common::RuntimeException( "Unsupported storage option for node table: " + entry->getStorage()); @@ -147,6 +153,15 @@ void StorageManager::addRelTable(RelGroupCatalogEntry* entry, const RelTableCata tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, info.nodePair.dstTableID, this, &memoryManager, *entry->getScanFunction(), std::move(entry->getScanBindData().value())); + } else if (!entry->getStorageFormat().empty()) { + if (TableOptionConstants::isIceBugDiskFormat(entry->getStorageFormat())) { + // Create icebug-disk-backed rel table + tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, + info.nodePair.dstTableID, this, &memoryManager, context); + } else { + throw common::RuntimeException( + "Unsupported storage format option for rel table: " + entry->getStorageFormat()); + } } else if (!entry->getStorage().empty()) { if (entry->getStorage().substr(0, 8) == "arrow://") { std::string arrowId = entry->getStorage().substr(8); @@ -175,10 +190,6 @@ void StorageManager::addRelTable(RelGroupCatalogEntry* entry, const RelTableCata tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, info.nodePair.dstTableID, this, &memoryManager, fromNodeTable, toNodeTable, std::move(schemaCopy), std::move(arraysCopy), arrowId); - } else if (TableOptionConstants::isIceBugDiskStorage(entry->getStorage())) { - // Create icebug-disk-backed rel table - tables[info.oid] = std::make_unique(entry, info.nodePair.srcTableID, - info.nodePair.dstTableID, this, &memoryManager, context); } else { throw common::RuntimeException( "Unsupported storage option for rel table: " + entry->getStorage()); @@ -446,7 +457,7 @@ void StorageManager::deserialize(main::ClientContext* context, const Catalog* ca auto tableEntry = catalog->getTableCatalogEntry(&DUMMY_TRANSACTION, tableID) ->ptrCast(); tableNameCache[tableID] = tableEntry->getName(); - if (!tableEntry->getStorage().empty()) { + if (TableOptionConstants::isIceBugDiskFormat(tableEntry->getStorageFormat())) { // Create icebug-disk-backed node table tables[tableID] = std::make_unique(this, tableEntry, &memoryManager, context); @@ -475,8 +486,7 @@ void StorageManager::deserialize(main::ClientContext* context, const Catalog* ca for (auto k = 0u; k < numInnerRelTables; k++) { RelTableCatalogInfo info = RelTableCatalogInfo::deserialize(deSer); DASSERT(!tables.contains(info.oid)); - if (!relGroupEntry->getStorage().empty() && - TableOptionConstants::isIceBugDiskStorage(relGroupEntry->getStorage())) { + if (TableOptionConstants::isIceBugDiskFormat(relGroupEntry->getStorageFormat())) { // Create icebug-disk-backed rel table tables[info.oid] = std::make_unique(relGroupEntry, info.nodePair.srcTableID, diff --git a/src/storage/table/ice_disk_node_table.cpp b/src/storage/table/ice_disk_node_table.cpp index e1e35184c4..0c5ebac24b 100644 --- a/src/storage/table/ice_disk_node_table.cpp +++ b/src/storage/table/ice_disk_node_table.cpp @@ -30,13 +30,11 @@ IceDiskNodeTable::IceDiskNodeTable(const StorageManager* storageManager, main::ClientContext* context) : ColumnarNodeTableBase{storageManager, nodeTableEntry, memoryManager, std::make_unique()} { - auto file = IceDiskUtils::constructNodeTablePath( - IceDiskUtils::getBasePath(nodeTableEntry->getStorage()), nodeTableEntry->getName(), - ".parquet"); - const auto dbDir = - std::filesystem::path(storageManager->getDatabasePath()).parent_path().string(); - parquetFilePath = - IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, file, context); + auto resolvedPath = common::VirtualFileSystem::resolvePath(context, + IceDiskUtils::constructNodeTablePath(nodeTableEntry->getStorage(), + nodeTableEntry->getName(), ".parquet")); + IceDiskUtils::checkVersionCompatibility(context, resolvedPath); + parquetFilePath = resolvedPath; } void IceDiskNodeTable::initializeScanCoordination(const transaction::Transaction* transaction) { @@ -74,9 +72,8 @@ void IceDiskNodeTable::initScanState(Transaction* transaction, TableScanState& s std::vector columnSkips; try { - auto resolvedPath = VirtualFileSystem::resolvePath(context, parquetFilePath); iceDiskScanState.parquetReader = - std::make_unique(resolvedPath, columnSkips, context); + std::make_unique(parquetFilePath, columnSkips, context); iceDiskScanState.initialized = true; } catch (const std::exception& e) { throw RuntimeException("Failed to initialize parquet reader for file '" + @@ -99,8 +96,7 @@ common::node_group_idx_t IceDiskNodeTable::getNumBatches(const Transaction* tran std::vector columnSkips; try { - auto resolvedPath = VirtualFileSystem::resolvePath(context, parquetFilePath); - auto tempReader = std::make_unique(resolvedPath, columnSkips, context); + auto tempReader = std::make_unique(parquetFilePath, columnSkips, context); return tempReader->getNumRowGroups(); } catch (const std::exception& e) { return 1; // Fallback @@ -331,8 +327,7 @@ row_idx_t IceDiskNodeTable::getTotalRowCount(const Transaction* transaction) con std::vector columnSkips; try { - auto resolvedPath = VirtualFileSystem::resolvePath(context, parquetFilePath); - auto tempReader = std::make_unique(resolvedPath, columnSkips, context); + auto tempReader = std::make_unique(parquetFilePath, columnSkips, context); if (!tempReader) { return 0; } diff --git a/src/storage/table/ice_disk_rel_table.cpp b/src/storage/table/ice_disk_rel_table.cpp index b452b700c7..a87c68eac7 100644 --- a/src/storage/table/ice_disk_rel_table.cpp +++ b/src/storage/table/ice_disk_rel_table.cpp @@ -46,15 +46,16 @@ IceDiskRelTable::IceDiskRelTable(RelGroupCatalogEntry* relGroupEntry, table_id_t table_id_t toTableID, const StorageManager* storageManager, MemoryManager* memoryManager, main::ClientContext* context) : ColumnarRelTableBase{relGroupEntry, fromTableID, toTableID, storageManager, memoryManager} { - const auto base = IceDiskUtils::getBasePath(relGroupEntry->getStorage()); - auto paths = IceDiskUtils::constructCSRPaths(base, relGroupEntry->getName(), ".parquet"); - - const auto dbDir = - std::filesystem::path(storageManager->getDatabasePath()).parent_path().string(); - indicesFilePath = IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, - paths.indices, context); - indptrFilePath = IceDiskUtils::checkVersionCompatibility(storageManager->getVFS(), dbDir, - paths.indptr, context); + auto paths = IceDiskUtils::constructCSRPaths(relGroupEntry->getStorage(), + relGroupEntry->getName(), ".parquet"); + + auto resolvedIndicesPath = VirtualFileSystem::resolvePath(context, paths.indices); + IceDiskUtils::checkVersionCompatibility(context, resolvedIndicesPath); + + auto resolvedIndptrPath = VirtualFileSystem::resolvePath(context, paths.indptr); + IceDiskUtils::checkVersionCompatibility(context, resolvedIndptrPath); + indicesFilePath = resolvedIndicesPath; + indptrFilePath = resolvedIndptrPath; } void IceDiskRelTable::initScanState(Transaction* transaction, TableScanState& scanState, @@ -72,16 +73,14 @@ void IceDiskRelTable::initScanState(Transaction* transaction, TableScanState& sc if (!iceDiskScanState.indicesReader) { std::vector columnSkips; // Read all columns auto context = transaction->getClientContext(); - auto resolvedPath = VirtualFileSystem::resolvePath(context, indicesFilePath); iceDiskScanState.indicesReader = - std::make_unique(resolvedPath, columnSkips, context); + std::make_unique(indicesFilePath, columnSkips, context); } if (!indptrFilePath.empty() && !iceDiskScanState.indptrReader) { std::vector columnSkips; // Read all columns auto context = transaction->getClientContext(); - auto resolvedPath = VirtualFileSystem::resolvePath(context, indptrFilePath); iceDiskScanState.indptrReader = - std::make_unique(resolvedPath, columnSkips, context); + std::make_unique(indptrFilePath, columnSkips, context); } // Load shared indptr data - thread-safe to read @@ -118,8 +117,7 @@ void IceDiskRelTable::initializeParquetReaders(Transaction* transaction) const { if (!indicesReader) { std::vector columnSkips; // Read all columns auto context = transaction->getClientContext(); - auto resolvedPath = VirtualFileSystem::resolvePath(context, indicesFilePath); - indicesReader = std::make_unique(resolvedPath, columnSkips, context); + indicesReader = std::make_unique(indicesFilePath, columnSkips, context); } } } @@ -130,8 +128,7 @@ void IceDiskRelTable::initializeIndptrReader(Transaction* transaction) const { if (!indptrReader) { std::vector columnSkips; // Read all columns auto context = transaction->getClientContext(); - auto resolvedPath = VirtualFileSystem::resolvePath(context, indptrFilePath); - indptrReader = std::make_unique(resolvedPath, columnSkips, context); + indptrReader = std::make_unique(indptrFilePath, columnSkips, context); } } } diff --git a/test/storage/ice_disk_utils_test.cpp b/test/storage/ice_disk_utils_test.cpp index 3887f1d373..83693bd6d3 100644 --- a/test/storage/ice_disk_utils_test.cpp +++ b/test/storage/ice_disk_utils_test.cpp @@ -5,11 +5,16 @@ #include "common/exception/io.h" #include "common/exception/runtime.h" #include "common/file_system/virtual_file_system.h" +#include "graph_test/private_graph_test.h" #include "gtest/gtest.h" +#include "main/client_context.h" +#include "main/connection.h" +#include "main/database.h" #include "storage/table/ice_disk_utils.h" #include "test_helper/test_helper.h" using namespace lbug::common; +using namespace lbug::main; using namespace lbug::storage; using namespace lbug::testing; @@ -18,30 +23,6 @@ static const std::string FIXTURES_DIR = static const std::string DEMO_DB_ICEBUG_DISK = TestHelper::appendLbugRootPath("dataset/demo-db/icebug-disk"); -// ───────────────────────────────────────────────────────────── -// getBasePath -// ───────────────────────────────────────────────────────────── - -TEST(IceDiskUtils_GetBasePath, PrefixOnly) { - EXPECT_EQ("", IceDiskUtils::getBasePath("icebug-disk")); -} - -TEST(IceDiskUtils_GetBasePath, PrefixWithColonOnly) { - EXPECT_EQ("", IceDiskUtils::getBasePath("icebug-disk:")); -} - -TEST(IceDiskUtils_GetBasePath, PrefixWithAbsolutePath) { - EXPECT_EQ("/some/path", IceDiskUtils::getBasePath("icebug-disk:/some/path")); -} - -TEST(IceDiskUtils_GetBasePath, PrefixWithRelativePath) { - EXPECT_EQ("rel/path", IceDiskUtils::getBasePath("icebug-disk:rel/path")); -} - -TEST(IceDiskUtils_GetBasePath, PrefixWithDot) { - EXPECT_EQ(".", IceDiskUtils::getBasePath("icebug-disk:.")); -} - // ───────────────────────────────────────────────────────────── // joinPath // ───────────────────────────────────────────────────────────── @@ -61,6 +42,16 @@ TEST(IceDiskUtils_JoinPath, BaseWithBackslash) { EXPECT_EQ("base\\file.parquet", IceDiskUtils::joinPath("base\\", "file.parquet")); } +TEST(IceDiskUtils_JoinPath, S3URI) { + EXPECT_EQ("s3://bucket/prefix/file.parquet", + IceDiskUtils::joinPath("s3://bucket/prefix", "file.parquet")); +} + +TEST(IceDiskUtils_JoinPath, HttpsURI) { + EXPECT_EQ("https://host/path/file.parquet", + IceDiskUtils::joinPath("https://host/path", "file.parquet")); +} + // ───────────────────────────────────────────────────────────── // constructNodeTablePath // ───────────────────────────────────────────────────────────── @@ -73,6 +64,11 @@ TEST(IceDiskUtils_ConstructNodeTablePath, WithDir) { IceDiskUtils::constructNodeTablePath("/some/dir", "user", ".parquet")); } +TEST(IceDiskUtils_ConstructNodeTablePath, S3URI) { + EXPECT_EQ("s3://bucket/data/nodes_user.parquet", + IceDiskUtils::constructNodeTablePath("s3://bucket/data", "user", ".parquet")); +} + // ───────────────────────────────────────────────────────────── // constructCSRPaths // ───────────────────────────────────────────────────────────── @@ -88,63 +84,48 @@ TEST(IceDiskUtils_ConstructCSRPaths, WithDir) { EXPECT_EQ("/some/dir/indptr_knows.parquet", paths.indptr); } -// ───────────────────────────────────────────────────────────── -// resolveIceDiskPath -// ───────────────────────────────────────────────────────────── -TEST(IceDiskUtils_ResolveIceDiskPath, AbsolutePathUnchanged) { - EXPECT_EQ("/abs/path/file.parquet", - IceDiskUtils::resolveIceDiskPath("/abs/path/file.parquet", "/dbdir")); -} - -TEST(IceDiskUtils_ResolveIceDiskPath, RelativePathJoinedWithDbDir) { - auto result = IceDiskUtils::resolveIceDiskPath("rel/file.parquet", "/dbdir"); - // std::filesystem normalizes the path, so check it ends with rel/file.parquet - EXPECT_TRUE(result.find("rel/file.parquet") != std::string::npos); - EXPECT_TRUE(std::filesystem::path(result).is_absolute()); -} - -TEST(IceDiskUtils_ResolveIceDiskPath, EmptyPathUsesDbDir) { - auto result = IceDiskUtils::resolveIceDiskPath("", "/dbdir"); - EXPECT_TRUE(std::filesystem::path(result).is_absolute()); -} - -TEST(IceDiskUtils_ResolveIceDiskPath, DotPathNormalized) { - auto result = IceDiskUtils::resolveIceDiskPath(".", "/dbdir"); - // "." relative to /dbdir should resolve to /dbdir (with possible trailing slash on some impls) - EXPECT_TRUE(result == "/dbdir" || result == "/dbdir/"); +TEST(IceDiskUtils_ConstructCSRPaths, S3URI) { + auto paths = IceDiskUtils::constructCSRPaths("s3://bucket/data", "follows", ".parquet"); + EXPECT_EQ("s3://bucket/data/indices_follows.parquet", paths.indices); + EXPECT_EQ("s3://bucket/data/indptr_follows.parquet", paths.indptr); } // ───────────────────────────────────────────────────────────── // checkVersionCompatibility // ───────────────────────────────────────────────────────────── -class IceDiskCheckVersionTest : public ::testing::Test { +class IceDiskCheckVersionTest : public EmptyDBTest { protected: - VirtualFileSystem vfs; + void SetUp() override { + EmptyDBTest::SetUp(); + createDBAndConn(); + context = conn->getClientContext(); + } + + ClientContext* context = nullptr; const std::string dbDir = FIXTURES_DIR; }; -TEST_F(IceDiskCheckVersionTest, NullVfs) { - EXPECT_THROW(IceDiskUtils::checkVersionCompatibility(nullptr, dbDir, +TEST_F(IceDiskCheckVersionTest, NullContext) { + EXPECT_THROW(IceDiskUtils::checkVersionCompatibility(nullptr, DEMO_DB_ICEBUG_DISK + "/nodes_person.parquet"), RuntimeException); } TEST_F(IceDiskCheckVersionTest, FileDoesNotExist) { - EXPECT_THROW(IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + EXPECT_THROW(IceDiskUtils::checkVersionCompatibility(context, FIXTURES_DIR + "/nodes_nonexistent.parquet"), IOException); } TEST_F(IceDiskCheckVersionTest, NotAParquetFile) { - EXPECT_THROW(IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + EXPECT_THROW(IceDiskUtils::checkVersionCompatibility(context, FIXTURES_DIR + "/nodes_notparquet.parquet"), CopyException); } TEST_F(IceDiskCheckVersionTest, MissingVersionKey) { try { - IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, - FIXTURES_DIR + "/nodes_noversion.parquet"); + IceDiskUtils::checkVersionCompatibility(context, FIXTURES_DIR + "/nodes_noversion.parquet"); FAIL() << "Expected RuntimeException for missing version key"; } catch (const RuntimeException& e) { EXPECT_TRUE(std::string(e.what()).find("missing icebug_disk_version") != std::string::npos); @@ -153,7 +134,7 @@ TEST_F(IceDiskCheckVersionTest, MissingVersionKey) { TEST_F(IceDiskCheckVersionTest, WrongVersionValue) { try { - IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + IceDiskUtils::checkVersionCompatibility(context, FIXTURES_DIR + "/nodes_wrongversion.parquet"); FAIL() << "Expected RuntimeException for wrong version"; } catch (const RuntimeException& e) { @@ -164,11 +145,11 @@ TEST_F(IceDiskCheckVersionTest, WrongVersionValue) { TEST_F(IceDiskCheckVersionTest, UppercaseVersionSucceeds) { // "V1" should match "v1" case-insensitively - EXPECT_NO_THROW(IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + EXPECT_NO_THROW(IceDiskUtils::checkVersionCompatibility(context, FIXTURES_DIR + "/nodes_upperversion.parquet")); } TEST_F(IceDiskCheckVersionTest, ValidV1Succeeds) { - EXPECT_NO_THROW(IceDiskUtils::checkVersionCompatibility(&vfs, dbDir, + EXPECT_NO_THROW(IceDiskUtils::checkVersionCompatibility(context, DEMO_DB_ICEBUG_DISK + "/nodes_user.parquet")); } diff --git a/test/test_files/ice_disk/ice_disk_invalid_storage.test b/test/test_files/ice_disk/ice_disk_invalid_storage.test index f255336c6f..255a290d19 100644 --- a/test/test_files/ice_disk/ice_disk_invalid_storage.test +++ b/test/test_files/ice_disk/ice_disk_invalid_storage.test @@ -4,31 +4,29 @@ -CASE IceDiskNonExistentDirNodeFails -LOG IceDiskNonExistentDirMissingParquet --STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (storage = 'icebug-disk:/nonexistent/dir/that/does/not/exist') +-STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (storage = '/nonexistent/dir/that/does/not/exist', format = 'icebug-disk') ---- error(regex) .*[Nn]o such file.*|.*[Cc]annot open.*|.*nodes_t\.parquet.* --CASE IceDiskURINodeFails +-CASE IceDiskRemoteURINodeFails --LOG IceDiskURIMissingParquet --STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (storage = 'icebug-disk:https://example.com/path') +-LOG IceDiskRemoteURIMissingParquet +-STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (storage = 'https://example.com/path', format = 'icebug-disk') ---- error(regex) .*[Nn]o such file.*|.*[Cc]annot open.*|.*nodes_t\.parquet.* --CASE IceDiskPrefixOnlyRelFails +-CASE IceDiskS3URINodeFails --LOG IceDiskPrefixOnlyRelMissingParquet --STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (storage = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') ----- ok --STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) WITH (storage = 'icebug-disk') +-LOG IceDiskS3URIMissingParquet +-STATEMENT CREATE NODE TABLE t(id INT64 PRIMARY KEY) WITH (storage = 's3://my-bucket/data', format = 'icebug-disk') ---- error(regex) -.*[Nn]o such file.*|.*[Cc]annot open.*|.*indices_rel\.parquet.* +.*[Nn]o such file.*|.*[Cc]annot open.*|.*nodes_t\.parquet.* --CASE IceDiskPrefixWithColonRelFails +-CASE IceDiskNoStorageRelFails --LOG IceDiskPrefixColonRelMissingParquet --STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (storage = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') +-LOG IceDiskNoStorageRelMissingParquet +-STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (storage = '${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures', format = 'icebug-disk') ---- ok --STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) WITH (storage = 'icebug-disk:') +-STATEMENT CREATE REL TABLE rel(FROM upperversion TO upperversion) WITH (format = 'icebug-disk') ---- error(regex) .*[Nn]o such file.*|.*[Cc]annot open.*|.*indices_rel\.parquet.* diff --git a/test/test_files/ice_disk/ice_disk_mix_tables.test b/test/test_files/ice_disk/ice_disk_mix_tables.test index 296bdfd5ae..dd88abbc63 100644 --- a/test/test_files/ice_disk/ice_disk_mix_tables.test +++ b/test/test_files/ice_disk/ice_disk_mix_tables.test @@ -8,14 +8,14 @@ ---- ok -LOG IceDiskRelWithRegularNodeFails --STATEMENT CREATE REL TABLE rel(FROM person TO person) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') +-STATEMENT CREATE REL TABLE rel(FROM person TO person) WITH (storage = '${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures', format = 'icebug-disk') ---- error Binder exception: Cannot mix icebug-disk tables with non-icebug-disk tables in CREATE REL TABLE. -CASE NodeIceDiskRelRegular -LOG CreateIceDiskNodeTable --STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (STORAGE = 'icebug-disk:${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures') +-STATEMENT CREATE NODE TABLE upperversion(id INT64 PRIMARY KEY) WITH (storage = '${LBUG_ROOT_DIRECTORY}/dataset/ice-disk-test/fixtures', format = 'icebug-disk') ---- ok -LOG CreateRegularNodeTable diff --git a/test/test_helper/test_helper.cpp b/test/test_helper/test_helper.cpp index eb16e829a0..8d0e92c751 100644 --- a/test/test_helper/test_helper.cpp +++ b/test/test_helper/test_helper.cpp @@ -89,46 +89,51 @@ void TestHelper::executeScript(const std::string& cypherScript, Connection& conn line.replace(line.find(csvFilePath), csvFilePath.length(), fullPath); } - // Also handle storage = 'path' for icebug-disk tables - std::vector storagePaths; - size_t storageIndex = 0; - while (true) { - size_t start = line.find("storage = \"", storageIndex); - if (start == std::string::npos) { - break; - } - start += 11; // length of "storage = \"" - size_t end = line.find("\"", start); - if (end == std::string::npos) { - break; - } - std::string storagePath = line.substr(start, end - start); - storagePaths.push_back(storagePath); - storageIndex = end + 1; - } - for (auto& storagePath : storagePaths) { - static constexpr std::string_view iceBugPrefix = "icebug-disk"; - - if (storagePath.starts_with(iceBugPrefix)) { - // Strip "icebug-disk" prefix and optional ':' separator. - std::string basePath = storagePath.substr(iceBugPrefix.size()); - if (!basePath.empty() && basePath[0] == ':') { - basePath = basePath.substr(1); - } - // Resolve empty or relative paths relative to the schema's directory. - // Absolute paths and object-store URLs (contain "://") are left unchanged. - if (basePath.empty()) { - basePath = normalizePathForCypher(cypherDir.string()); - } else if (basePath.find("://") == std::string::npos && - std::filesystem::path(basePath).is_relative()) { - basePath = normalizePathForCypher((cypherDir / basePath).string()); - } - std::string resolvedStorage = std::string(iceBugPrefix) + ":" + basePath; - size_t pos = line.find(storagePath); - if (pos != std::string::npos) { - line.replace(pos, storagePath.length(), resolvedStorage); + // handle icebug-disk storage format: resolve storage = "" for icebug-disk tables + { + size_t fmtStart = line.find("format = \"", 0); + if (fmtStart != std::string::npos) { + fmtStart += 10; // length of "format = \"" + size_t fmtEnd = line.find("\"", fmtStart); + if (fmtEnd != std::string::npos) { + std::string storageFormat = line.substr(fmtStart, fmtEnd - fmtStart); + if (storageFormat.find("icebug-disk") != std::string::npos) { + std::vector storagePaths; + size_t storageIndex = 0; + while (true) { + size_t sStart = line.find("storage = \"", storageIndex); + if (sStart == std::string::npos) { + break; + } + sStart += 11; // length of "storage = \"" + size_t sEnd = line.find("\"", sStart); + if (sEnd == std::string::npos) { + break; + } + storagePaths.push_back(line.substr(sStart, sEnd - sStart)); + storageIndex = sEnd + 1; + } + for (auto& storagePath : storagePaths) { + // Remote URIs (e.g. s3://, https://) are passed through unchanged. + if (storagePath.find("://") != std::string::npos) { + continue; + } + auto fullPath = storagePath; + if (std::filesystem::path(storagePath).is_relative()) { + if (std::filesystem::path(storagePath).parent_path().empty()) { + fullPath = (cypherDir / storagePath).string(); + } else { + fullPath = appendLbugRootPath(storagePath); + } + } + fullPath = normalizePathForCypher(std::move(fullPath)); + size_t pos = line.find(storagePath); + if (pos != std::string::npos) { + line.replace(pos, storagePath.length(), fullPath); + } + } + } } - continue; } } #ifdef __STATIC_LINK_EXTENSION_TEST__ From 4efda9c010b82433aa3f6f217628258d625fe1a6 Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Wed, 13 May 2026 07:35:02 +0530 Subject: [PATCH 09/10] update dataset submodule --- dataset | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dataset b/dataset index 30632c3f24..1eb7993b8c 160000 --- a/dataset +++ b/dataset @@ -1 +1 @@ -Subproject commit 30632c3f24cffe473c992ae8d29f278b654807a0 +Subproject commit 1eb7993b8caeed3b75cb931698522640b9557481 From 3b089cebcf84851d3b5f8a0347e5b7f9d587a341 Mon Sep 17 00:00:00 2001 From: Ally Heev Date: Wed, 13 May 2026 07:35:26 +0530 Subject: [PATCH 10/10] revert toCypher changes --- .../catalog_entry/node_table_catalog_entry.cpp | 14 +------------- .../catalog_entry/rel_group_catalog_entry.cpp | 12 +----------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/catalog/catalog_entry/node_table_catalog_entry.cpp b/src/catalog/catalog_entry/node_table_catalog_entry.cpp index 588e332852..350fcf725f 100644 --- a/src/catalog/catalog_entry/node_table_catalog_entry.cpp +++ b/src/catalog/catalog_entry/node_table_catalog_entry.cpp @@ -48,20 +48,8 @@ std::unique_ptr NodeTableCatalogEntry::deserialize( } std::string NodeTableCatalogEntry::toCypher(const ToCypherInfo& /*info*/) const { - std::stringstream ss; - ss << std::format("CREATE NODE TABLE `{}` ({} PRIMARY KEY(`{}`))", getName(), + return std::format("CREATE NODE TABLE `{}` ({} PRIMARY KEY(`{}`));", getName(), propertyCollection.toCypher(), primaryKeyName); - - if (!storage.empty()) { - ss << std::format(" WITH (STORAGE = '{}'", storage); - if (!storageFormat.empty()) { - ss << std::format(", FORMAT = '{}'", storageFormat); - } - ss << ")"; - } - - ss << ";"; - return ss.str(); } std::optional NodeTableCatalogEntry::getScanFunction() const { diff --git a/src/catalog/catalog_entry/rel_group_catalog_entry.cpp b/src/catalog/catalog_entry/rel_group_catalog_entry.cpp index 8e637d6f46..86acd13dcd 100644 --- a/src/catalog/catalog_entry/rel_group_catalog_entry.cpp +++ b/src/catalog/catalog_entry/rel_group_catalog_entry.cpp @@ -177,17 +177,7 @@ std::string RelGroupCatalogEntry::toCypher(const ToCypherInfo& info) const { getFromToStr(relTableInfos[i].nodePair, catalog, transaction, storage)); } ss << ", " << propertyCollection.toCypher() << RelMultiplicityUtils::toString(srcMultiplicity) - << "_" << RelMultiplicityUtils::toString(dstMultiplicity) << ")"; - - if (!storage.empty()) { - ss << std::format(" WITH (STORAGE = '{}'", storage); - if (!storageFormat.empty()) { - ss << std::format(", FORMAT = '{}'", storageFormat); - } - ss << ")"; - } - ss << ";"; - + << "_" << RelMultiplicityUtils::toString(dstMultiplicity) << ");"; return ss.str(); }