diff --git a/src/binder/bind/bind_copy.cpp b/src/binder/bind/bind_copy.cpp index f5c500c6fe2..1fe4f1bd1fc 100644 --- a/src/binder/bind/bind_copy.cpp +++ b/src/binder/bind/bind_copy.cpp @@ -79,10 +79,18 @@ std::unique_ptr Binder::bindCopyFromClause(const Statement& stat auto& copyStatement = (CopyFrom&)statement; auto catalogContent = catalog.getReadOnlyVersion(); auto tableName = copyStatement.getTableName(); - validateNodeRelTableExist(tableName); + validateTableExist(tableName); // Bind to table schema. auto tableID = catalogContent->getTableID(tableName); auto tableSchema = catalogContent->getTableSchema(tableID); + switch (tableSchema->tableType) { + case TableType::REL_GROUP: + case TableType::RDF: { + throw BinderException(StringUtils::string_format("Cannot copy into {} table with type {}.", tableName, TableTypeUtils::toString(tableSchema->tableType))); + } + default: + break; + } auto csvReaderConfig = bindParsingOptions(copyStatement.getParsingOptionsRef()); auto filePaths = bindFilePaths(copyStatement.getFilePaths()); auto fileType = bindFileType(filePaths); diff --git a/src/binder/bind/bind_ddl.cpp b/src/binder/bind/bind_ddl.cpp index b231f692e9c..4aeba7ca5d9 100644 --- a/src/binder/bind/bind_ddl.cpp +++ b/src/binder/bind/bind_ddl.cpp @@ -4,6 +4,7 @@ #include "binder/ddl/bound_drop_table.h" #include "catalog/node_table_schema.h" #include "catalog/rel_table_schema.h" +#include "catalog/rel_table_group_schema.h" #include "common/exception/binder.h" #include "common/string_utils.h" #include "parser/ddl/alter.h" @@ -219,11 +220,35 @@ std::unique_ptr Binder::bindCreateTable(const parser::Statement& std::unique_ptr Binder::bindDropTable(const Statement& statement) { auto& dropTable = reinterpret_cast(statement); auto tableName = dropTable.getTableName(); - validateNodeRelTableExist(tableName); + validateTableExist(tableName); auto catalogContent = catalog.getReadOnlyVersion(); auto tableID = catalogContent->getTableID(tableName); - if (catalogContent->containsNodeTable(tableName)) { - validateNodeTableHasNoEdge(catalog, tableID); + auto tableSchema = catalogContent->getTableSchema(tableID); + switch (tableSchema->tableType) { + case TableType::NODE: { + for (auto& schema : catalogContent->getRelTableSchemas()) { + auto relTableSchema = reinterpret_cast(schema); + if (relTableSchema->isSrcOrDstTable(tableID)) { + throw BinderException(StringUtils::string_format( + "Cannot delete node table {} referenced by rel table {}.", tableSchema->tableName, + relTableSchema->tableName)); + } + } + } break ; + case TableType::REL: { + for (auto& schema : catalogContent->getRelTableGroupSchemas()) { + auto relTableGroupSchema = reinterpret_cast(schema); + for (auto& relTableID : relTableGroupSchema->getRelTableIDs()) { + if (relTableID == tableSchema->getTableID()) { + throw BinderException(StringUtils::string_format( + "Cannot delete rel table {} referenced by rel group {}.", tableSchema->tableName, + relTableGroupSchema->tableName)); + } + } + } + } + default: + break ; } return make_unique(tableID, tableName); } @@ -255,7 +280,7 @@ std::unique_ptr Binder::bindRenameTable(const Statement& stateme auto tableName = info->tableName; auto newName = extraInfo->newName; auto catalogContent = catalog.getReadOnlyVersion(); - validateNodeRelTableExist(tableName); + validateTableExist(tableName); if (catalogContent->containsTable(newName)) { throw BinderException("Table: " + newName + " already exists."); } @@ -280,6 +305,17 @@ static void validatePropertyNotExist(TableSchema* tableSchema, const std::string } } +static void validatePropertyDDLOnTable(TableSchema* tableSchema, const std::string& ddlOperation) { + switch (tableSchema->tableType) { + case TableType::REL_GROUP: + case TableType::RDF: { + throw BinderException(StringUtils::string_format("Cannot {} property on table {} with type {}.", ddlOperation, tableSchema->tableName, TableTypeUtils::toString(tableSchema->tableType))); + } + default: + return; + } +} + std::unique_ptr Binder::bindAddProperty(const Statement& statement) { auto& alter = reinterpret_cast(statement); auto info = alter.getInfo(); @@ -287,10 +323,11 @@ std::unique_ptr Binder::bindAddProperty(const Statement& stateme auto tableName = info->tableName; auto dataType = bindDataType(extraInfo->dataType); auto propertyName = extraInfo->propertyName; - validateNodeRelTableExist(tableName); + validateTableExist(tableName); auto catalogContent = catalog.getReadOnlyVersion(); auto tableID = catalogContent->getTableID(tableName); auto tableSchema = catalogContent->getTableSchema(tableID); + validatePropertyDDLOnTable(tableSchema, "add"); validatePropertyNotExist(tableSchema, propertyName); if (dataType->getLogicalTypeID() == LogicalTypeID::SERIAL) { throw BinderException("Serial property in node table must be the primary key."); @@ -310,10 +347,11 @@ std::unique_ptr Binder::bindDropProperty(const Statement& statem auto extraInfo = reinterpret_cast(info->extraInfo.get()); auto tableName = info->tableName; auto propertyName = extraInfo->propertyName; - validateNodeRelTableExist(tableName); + validateTableExist(tableName); auto catalogContent = catalog.getReadOnlyVersion(); auto tableID = catalogContent->getTableID(tableName); auto tableSchema = catalogContent->getTableSchema(tableID); + validatePropertyDDLOnTable(tableSchema, "drop"); validatePropertyExist(tableSchema, propertyName); auto propertyID = tableSchema->getPropertyID(propertyName); if (tableSchema->getTableType() == TableType::NODE && @@ -333,10 +371,11 @@ std::unique_ptr Binder::bindRenameProperty(const Statement& stat auto tableName = info->tableName; auto propertyName = extraInfo->propertyName; auto newName = extraInfo->newName; - validateNodeRelTableExist(tableName); + validateTableExist(tableName); auto catalogContent = catalog.getReadOnlyVersion(); auto tableID = catalogContent->getTableID(tableName); auto tableSchema = catalogContent->getTableSchema(tableID); + validatePropertyDDLOnTable(tableSchema, "rename"); validatePropertyExist(tableSchema, propertyName); auto propertyID = tableSchema->getPropertyID(propertyName); validatePropertyNotExist(tableSchema, newName); diff --git a/src/binder/binder.cpp b/src/binder/binder.cpp index 4821bb84ec0..a55953cd408 100644 --- a/src/binder/binder.cpp +++ b/src/binder/binder.cpp @@ -172,24 +172,6 @@ void Binder::validateTableExist(const std::string& tableName) { } } -void Binder::validateNodeRelTableExist(const std::string& tableName) { - if (!catalog.getReadOnlyVersion()->containsNodeTable(tableName) && - !catalog.getReadOnlyVersion()->containsRelTable(tableName)) { - throw BinderException("Table " + tableName + " does not exist."); - } -} - -void Binder::validateNodeTableHasNoEdge(const Catalog& _catalog, table_id_t tableID) { - for (auto& tableSchema : _catalog.getReadOnlyVersion()->getRelTableSchemas()) { - auto relTableSchema = reinterpret_cast(tableSchema); - if (relTableSchema->isSrcOrDstTable(tableID)) { - throw BinderException(StringUtils::string_format( - "Cannot delete a node table with edges. It is on the edges of rel: {}.", - relTableSchema->tableName)); - } - } -} - std::string Binder::getUniqueExpressionName(const std::string& name) { return "_" + std::to_string(lastExpressionId++) + "_" + name; } diff --git a/src/catalog/catalog.cpp b/src/catalog/catalog.cpp index 774ad21251d..562806c9b09 100644 --- a/src/catalog/catalog.cpp +++ b/src/catalog/catalog.cpp @@ -58,23 +58,34 @@ common::table_id_t Catalog::addRelTableGroupSchema(const binder::BoundCreateTabl auto tableID = catalogContentForWriteTrx->addRelTableGroupSchema(info); auto relTableGroupSchema = (RelTableGroupSchema*)catalogContentForWriteTrx->getTableSchema(tableID); - // TODO(Ziyi): remove this when we can log variable size record. See also wal_record.h for (auto relTableID : relTableGroupSchema->getRelTableIDs()) { wal->logRelTableRecord(relTableID); } - wal->logRelTableGroupRecord(tableID, relTableGroupSchema->getRelTableIDs()); return tableID; } -common::table_id_t Catalog::addRdfGraphSchema(const binder::BoundCreateTableInfo& info) { +table_id_t Catalog::addRdfGraphSchema(const binder::BoundCreateTableInfo& info) { initCatalogContentForWriteTrxIfNecessary(); return catalogContentForWriteTrx->addRdfGraphSchema(info); } void Catalog::dropTableSchema(table_id_t tableID) { initCatalogContentForWriteTrxIfNecessary(); - catalogContentForWriteTrx->dropTableSchema(tableID); - wal->logDropTableRecord(tableID); + auto tableSchema = catalogContentForWriteTrx->getTableSchema(tableID); + switch (tableSchema->tableType) { + case TableType::REL_GROUP: { + auto relTableGroupSchema = reinterpret_cast(tableSchema); + auto relTableIDs = relTableGroupSchema->getRelTableIDs(); + catalogContentForWriteTrx->dropTableSchema(tableID); + for (auto relTableID : relTableIDs) { + wal->logDropTableRecord(relTableID); + } + } break; + default: { + catalogContentForWriteTrx->dropTableSchema(tableID); + wal->logDropTableRecord(tableID); + } + } } void Catalog::renameTable(table_id_t tableID, const std::string& newName) { diff --git a/src/catalog/catalog_content.cpp b/src/catalog/catalog_content.cpp index 0373d9eb7c2..1d96f63c864 100644 --- a/src/catalog/catalog_content.cpp +++ b/src/catalog/catalog_content.cpp @@ -130,8 +130,18 @@ std::vector CatalogContent::getTableSchemas( } void CatalogContent::dropTableSchema(table_id_t tableID) { - auto tableName = getTableName(tableID); - tableNameToIDMap.erase(tableName); + auto tableSchema = getTableSchema(tableID); + switch (tableSchema->tableType) { + case common::TableType::REL_GROUP: { + auto relTableGroupSchema = reinterpret_cast(tableSchema); + for (auto& relTableID : relTableGroupSchema->getRelTableIDs()) { + dropTableSchema(relTableID); + } + } break; + default: + break; + } + tableNameToIDMap.erase(tableSchema->tableName); tableSchemas.erase(tableID); } diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 0ce3661f69e..923437dfd21 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -20,6 +20,7 @@ add_library(kuzu_common type_utils.cpp utils.cpp string_utils.cpp + table_type.cpp ser_deser.cpp) target_link_libraries(kuzu_common Glob) diff --git a/src/common/table_type.cpp b/src/common/table_type.cpp new file mode 100644 index 00000000000..62b9e4f6e84 --- /dev/null +++ b/src/common/table_type.cpp @@ -0,0 +1,30 @@ +#include "common/table_type.h" +#include "common/exception/not_implemented.h" + +namespace kuzu { +namespace common { + +std::string TableTypeUtils::toString(TableType tableType) { + switch (tableType) { + case TableType::UNKNOWN: { + return "UNKNOWN"; + } + case TableType::NODE: { + return "NODE"; + } + case TableType::REL: { + return "REL"; + } + case TableType::RDF: { + return "RDF"; + } + case TableType::REL_GROUP: { + return "REL_GROUP"; + } + default: // LCOV_EXCL_START + throw NotImplementedException("TableTypeUtils::toString"); // LCOV_EXCL_STOP + } +} + +} +} diff --git a/src/function/table_functions.cpp b/src/function/table_functions.cpp index 4ec0b7bf653..d799174f84a 100644 --- a/src/function/table_functions.cpp +++ b/src/function/table_functions.cpp @@ -104,22 +104,7 @@ void ShowTablesFunction::tableFunc(std::pair morsel, for (auto i = 0u; i < numTablesToOutput; i++) { auto tableSchema = tables[morsel.first + i]; outputVectors[0]->setValue(i, tableSchema->tableName); - - std::string typeString; - switch (tableSchema->tableType) { - case TableType::NODE: { - typeString = "NODE"; - } break; - case TableType::REL: { - typeString = "REL"; - } break; - case TableType::RDF: { - typeString = "RDF"; - } break; - default: { - throw common::NotImplementedException{"ShowTablesFunction::tableFunc"}; - } - }; + std::string typeString = TableTypeUtils::toString(tableSchema->tableType); outputVectors[1]->setValue(i, typeString); outputVectors[2]->setValue(i, tableSchema->comment); } diff --git a/src/include/binder/binder.h b/src/include/binder/binder.h index 52693cd81c4..58da17145a4 100644 --- a/src/include/binder/binder.h +++ b/src/include/binder/binder.h @@ -255,11 +255,6 @@ class Binder { void validateTableType(common::table_id_t tableID, common::TableType expectedTableType); void validateTableExist(const std::string& tableName); - // TODO(Xiyang): remove this validation once we refactor DDL. - void validateNodeRelTableExist(const std::string& tableName); - - static void validateNodeTableHasNoEdge( - const catalog::Catalog& _catalog, common::table_id_t tableID); /*** helpers ***/ std::string getUniqueExpressionName(const std::string& name); diff --git a/src/include/catalog/catalog_content.h b/src/include/catalog/catalog_content.h index 863e923c3b2..9efbcf36e06 100644 --- a/src/include/catalog/catalog_content.h +++ b/src/include/catalog/catalog_content.h @@ -76,6 +76,9 @@ class CatalogContent { inline std::vector getRelTableSchemas() const { return getTableSchemas(common::TableType::REL); } + inline std::vector getRelTableGroupSchemas() const { + return getTableSchemas(common::TableType::REL_GROUP); + } std::vector getTableSchemas() const; std::vector getTableSchemas( diff --git a/src/include/common/table_type.h b/src/include/common/table_type.h index a0fcb337bbb..6a529417465 100644 --- a/src/include/common/table_type.h +++ b/src/include/common/table_type.h @@ -1,6 +1,7 @@ #pragma once #include +#include namespace kuzu { namespace common { @@ -13,5 +14,9 @@ enum class TableType : uint8_t { REL_GROUP = 4, }; +struct TableTypeUtils { + static std::string toString(TableType tableType); +}; + } // namespace common } // namespace kuzu diff --git a/src/include/storage/wal/wal.h b/src/include/storage/wal/wal.h index 11b0841556f..dedace7365d 100644 --- a/src/include/storage/wal/wal.h +++ b/src/include/storage/wal/wal.h @@ -103,8 +103,6 @@ class WAL : public BaseWALAndWALIterator { void logNodeTableRecord(common::table_id_t tableID); void logRelTableRecord(common::table_id_t tableID); - void logRelTableGroupRecord( - common::table_id_t tableID, std::vector relTableIDs); void logRdfGraphRecord(common::table_id_t rdfGraphID, common::table_id_t nodeTableID, common::table_id_t relTableID); diff --git a/src/include/storage/wal/wal_record.h b/src/include/storage/wal/wal_record.h index 51d1274bdd0..826919d2be8 100644 --- a/src/include/storage/wal/wal_record.h +++ b/src/include/storage/wal/wal_record.h @@ -331,17 +331,6 @@ struct RelTableRecord { inline bool operator==(const RelTableRecord& rhs) const { return tableID == rhs.tableID; } }; -struct RelTableGroupRecord { - common::table_id_t tableID; - // TODO(Ziyi): add this back when we can serialize variable size record. - // std::vector relTableRecords; - - RelTableGroupRecord() = default; - RelTableGroupRecord(common::table_id_t tableID) : tableID{tableID} {} - - bool operator==(const RelTableGroupRecord& other) const; -}; - struct RdfGraphRecord { common::table_id_t tableID; NodeTableRecord nodeTableRecord; @@ -456,7 +445,6 @@ struct WALRecord { CommitRecord commitRecord; NodeTableRecord nodeTableRecord; RelTableRecord relTableRecord; - RelTableGroupRecord relTableGroupRecord; RdfGraphRecord rdfGraphRecord; DiskOverflowFileNextBytePosRecord diskOverflowFileNextBytePosRecord; CopyNodeRecord copyNodeRecord; @@ -491,9 +479,6 @@ struct WALRecord { case WALRecordType::REL_TABLE_RECORD: { return relTableRecord == rhs.relTableRecord; } - case WALRecordType::REL_TABLE_GROUP_RECORD: { - return relTableGroupRecord == rhs.relTableGroupRecord; - } case WALRecordType::RDF_GRAPH_RECORD: { return rdfGraphRecord == rhs.rdfGraphRecord; } @@ -531,8 +516,6 @@ struct WALRecord { static WALRecord newCatalogRecord(); static WALRecord newNodeTableRecord(common::table_id_t tableID); static WALRecord newRelTableRecord(common::table_id_t tableID); - static WALRecord newRelTableGroupRecord( - common::table_id_t tableID, std::vector relTableIDs); static WALRecord newRdfGraphRecord(common::table_id_t rdfGraphID, common::table_id_t nodeTableID, common::table_id_t relTableID); static WALRecord newOverflowFileNextBytePosRecord( diff --git a/src/include/storage/wal_replayer.h b/src/include/storage/wal_replayer.h index ee42e50f93b..f020ed871b4 100644 --- a/src/include/storage/wal_replayer.h +++ b/src/include/storage/wal_replayer.h @@ -33,7 +33,6 @@ class WALReplayer { void replayNodeTableRecord(const WALRecord& walRecord); // TODO(Guodong/Ziyi) : fix this void replayRelTableRecord(const WALRecord& walRecord, bool isRdf = false); - void replayRelTableGroupRecord(const WALRecord& walRecord); void replayRdfGraphRecord(const WALRecord& walRecord); void replayOverflowFileNextBytePosRecord(const WALRecord& walRecord); void replayCopyNodeRecord(const WALRecord& walRecord); diff --git a/src/storage/wal/wal.cpp b/src/storage/wal/wal.cpp index 58d9869a29a..5ed258a7284 100644 --- a/src/storage/wal/wal.cpp +++ b/src/storage/wal/wal.cpp @@ -69,13 +69,6 @@ void WAL::logRelTableRecord(table_id_t tableID) { addNewWALRecordNoLock(walRecord); } -void WAL::logRelTableGroupRecord( - common::table_id_t tableID, std::vector relTableIDs) { - lock_t lck{mtx}; - WALRecord walRecord = WALRecord::newRelTableGroupRecord(tableID, std::move(relTableIDs)); - addNewWALRecordNoLock(walRecord); -} - void WAL::logRdfGraphRecord(table_id_t rdfGraphID, table_id_t nodeTableID, table_id_t relTableID) { lock_t lck{mtx}; WALRecord walRecord = WALRecord::newRdfGraphRecord(rdfGraphID, nodeTableID, relTableID); diff --git a/src/storage/wal/wal_record.cpp b/src/storage/wal/wal_record.cpp index 39306e569df..9323d73b793 100644 --- a/src/storage/wal/wal_record.cpp +++ b/src/storage/wal/wal_record.cpp @@ -142,10 +142,6 @@ std::string walRecordTypeToString(WALRecordType walRecordType) { } } -bool RelTableGroupRecord::operator==(const RelTableGroupRecord& other) const { - return tableID == other.tableID; -} - WALRecord WALRecord::newPageInsertOrUpdateRecord(StorageStructureID storageStructureID_, uint64_t pageIdxInOriginalFile, uint64_t pageIdxInWAL, bool isInsert) { WALRecord retVal; @@ -201,14 +197,6 @@ WALRecord WALRecord::newRelTableRecord(table_id_t tableID) { return retVal; } -WALRecord WALRecord::newRelTableGroupRecord( - table_id_t tableID, std::vector relTableIDs) { - WALRecord retVal; - retVal.recordType = WALRecordType::REL_TABLE_GROUP_RECORD; - retVal.relTableGroupRecord = RelTableGroupRecord(tableID); - return retVal; -} - WALRecord WALRecord::newRdfGraphRecord( table_id_t rdfGraphID, table_id_t nodeTableID, table_id_t relTableID) { WALRecord retVal; diff --git a/src/storage/wal_replayer.cpp b/src/storage/wal_replayer.cpp index 553a863baf5..0e5fef99468 100644 --- a/src/storage/wal_replayer.cpp +++ b/src/storage/wal_replayer.cpp @@ -74,9 +74,6 @@ void WALReplayer::replayWALRecord(WALRecord& walRecord) { case WALRecordType::REL_TABLE_RECORD: { replayRelTableRecord(walRecord); } break; - case WALRecordType::REL_TABLE_GROUP_RECORD: { - replayRelTableGroupRecord(walRecord); - } break; case WALRecordType::RDF_GRAPH_RECORD: { replayRdfGraphRecord(walRecord); } break; @@ -216,10 +213,6 @@ void WALReplayer::replayRelTableRecord(const WALRecord& walRecord, bool isRdf) { } } -void WALReplayer::replayRelTableGroupRecord(const WALRecord& walRecord) { - // DO NOTHING -} - void WALReplayer::replayRdfGraphRecord(const WALRecord& walRecord) { if (isCheckpoint) { WALRecord nodeTableWALRecord = {.recordType = WALRecordType::NODE_TABLE_RECORD, diff --git a/test/test_files/tinysnb/rel_group/basic.test b/test/test_files/tinysnb/rel_group/basic.test index 1a8c5d7fe19..923cc6b5e71 100644 --- a/test/test_files/tinysnb/rel_group/basic.test +++ b/test/test_files/tinysnb/rel_group/basic.test @@ -33,7 +33,66 @@ likes_personA_personB|personB|2|Bob likes_personB_personA|personA|2|Bob -STATEMENT COPY knows FROM "a.csv"; ---- error -Binder exception: Table knows does not exist. +Binder exception: Cannot copy into knows table with type REL_GROUP. +-STATEMENT DROP TABLE knows_personA_personB; +---- error +Binder exception: Cannot delete rel table knows_personA_personB referenced by rel group knows. +-STATEMENT CALL show_tables() RETURN *; +---- 10 +knows_personA_personB|REL| +knows_personA_personC|REL| +knows_personB_personC|REL| +knows|REL_GROUP| +likes_personA_personB|REL| +likes_personB_personA|REL| +likes|REL_GROUP| +personA|NODE| +personB|NODE| +personC|NODE| -STATEMENT DROP TABLE knows; +---- ok +-STATEMENT CALL show_tables() RETURN *; +---- 6 +likes_personA_personB|REL| +likes_personB_personA|REL| +likes|REL_GROUP| +personA|NODE| +personB|NODE| +personC|NODE| +-STATEMENT MATCH (a:personA)-[e:knows]->(b) RETURN COUNT(*) ---- error Binder exception: Table knows does not exist. +-STATEMENT MATCH (a:personA)-[e:knows_personA_personB]->(b) RETURN COUNT(*) +---- error +Binder exception: Table knows_personA_personB does not exist. +-STATEMENT ALTER TABLE likes_personA_personB RENAME TO likes_A_B; +---- ok +-STATEMENT ALTER TABLE likes RENAME TO hates; +---- ok +-STATEMENT CALL show_tables() RETURN *; +---- 6 +likes_A_B|REL| +likes_personB_personA|REL| +hates|REL_GROUP| +personA|NODE| +personB|NODE| +personC|NODE| +-STATEMENT MATCH (a:personA)-[e:hates]->(b:personB) WHERE a.ID = 0 RETURN label(e), label(b), b.* +---- 3 +likes_A_B|personB|2|Bob +likes_A_B|personB|3|Carol +likes_A_B|personB|5|Dan +-STATEMENT ALTER TABLE hates ADD age INT; +---- error +Binder exception: Cannot add property on table hates with type REL_GROUP. +-STATEMENT ALTER TABLE hates DROP age; +---- error +Binder exception: Cannot drop property on table hates with type REL_GROUP. +-STATEMENT ALTER TABLE hates RENAME age TO s; +---- error +Binder exception: Cannot rename property on table hates with type REL_GROUP. +-STATEMENT ALTER TABLE likes_A_B RENAME date TO s; +---- ok +-STATEMENT CALL table_info("likes_A_B") RETURN *; +---- 1 +1|s|DATE