Merge pull request #1194 from kuzudb/add-property-trx

add transaction to add property statement
kuzudb · Jan 23, 2023 · 299faf1 · 299faf1
2 parents 698ac85 + 3028bc3
commit 299faf1
Show file tree

Hide file tree

Showing 14 changed files with 459 additions and 144 deletions.
diff --git a/src/catalog/catalog.cpp b/src/catalog/catalog.cpp
@@ -387,7 +387,9 @@ void Catalog::dropProperty(table_id_t tableID, property_id_t propertyID) {
 void Catalog::addProperty(table_id_t tableID, string propertyName, DataType dataType) {
     initCatalogContentForWriteTrxIfNecessary();
     catalogContentForWriteTrx->getTableSchema(tableID)->addProperty(
-        std::move(propertyName), std::move(dataType));
+        propertyName, std::move(dataType));
+    wal->logAddPropertyRecord(tableID,
+        catalogContentForWriteTrx->getTableSchema(tableID)->getPropertyID(std::move(propertyName)));
 }
 
 } // namespace catalog

diff --git a/src/include/storage/store/rel_table.h b/src/include/storage/store/rel_table.h
@@ -132,7 +132,7 @@ class DirectedRelTableData {
     unique_ptr<ListsUpdateIteratorsForDirection> getListsUpdateIteratorsForDirection(
         table_id_t boundNodeTableID);
     void removeProperty(property_id_t propertyID);
-    void addProperty(Property& property, table_id_t tableID, WAL* wal);
+    void addProperty(Property& property, WAL* wal);
 
 private:
     void scanColumns(Transaction* transaction, RelTableScanState& scanState,
@@ -218,7 +218,7 @@ class RelTable {
         const shared_ptr<ValueVector>& dstNodeIDVector, const shared_ptr<ValueVector>& relIDVector,
         const shared_ptr<ValueVector>& propertyVector, uint32_t propertyID);
     void initEmptyRelsForNewNode(nodeID_t& nodeID);
-    void addProperty(Property property, table_id_t tableID);
+    void addProperty(Property property);
 
 private:
     inline void addToUpdatedRelTables() { wal->addToUpdatedRelTables(tableID); }

diff --git a/src/include/storage/wal/wal.h b/src/include/storage/wal/wal.h
@@ -29,8 +29,8 @@ class BaseWALAndWALIterator {
     BaseWALAndWALIterator() : BaseWALAndWALIterator(nullptr) {}
 
     explicit BaseWALAndWALIterator(shared_ptr<FileHandle> fileHandle)
-        : fileHandle{move(fileHandle)}, offsetInCurrentHeaderPage{INT64_MAX}, currentHeaderPageIdx{
-                                                                                  INT32_MAX} {
+        : fileHandle{std::move(fileHandle)}, offsetInCurrentHeaderPage{INT64_MAX},
+          currentHeaderPageIdx{INT32_MAX} {
         currentHeaderPageBuffer = make_unique<uint8_t[]>(WAL_HEADER_PAGE_SIZE);
     }
 
@@ -118,6 +118,8 @@ class WAL : public BaseWALAndWALIterator {
 
     void logDropPropertyRecord(table_id_t tableID, property_id_t propertyID);
 
+    void logAddPropertyRecord(table_id_t tableID, property_id_t propertyID);
+
     // Removes the contents of WAL file.
     void clearWAL();
 

diff --git a/src/include/storage/wal/wal_record.h b/src/include/storage/wal/wal_record.h
@@ -263,6 +263,7 @@ enum class WALRecordType : uint8_t {
     COPY_REL_RECORD = 8,
     DROP_TABLE_RECORD = 9,
     DROP_PROPERTY_RECORD = 10,
+    ADD_PROPERTY_RECORD = 11,
 };
 
 string walRecordTypeToString(WALRecordType walRecordType);
@@ -392,6 +393,20 @@ struct DropPropertyRecord {
     }
 };
 
+struct AddPropertyRecord {
+    table_id_t tableID;
+    property_id_t propertyID;
+
+    AddPropertyRecord() = default;
+
+    AddPropertyRecord(table_id_t tableID, property_id_t propertyID)
+        : tableID{tableID}, propertyID{propertyID} {}
+
+    inline bool operator==(const AddPropertyRecord& rhs) const {
+        return tableID == rhs.tableID && propertyID == rhs.propertyID;
+    }
+};
+
 struct WALRecord {
     WALRecordType recordType;
     union {
@@ -405,6 +420,7 @@ struct WALRecord {
         TableStatisticsRecord tableStatisticsRecord;
         DropTableRecord dropTableRecord;
         DropPropertyRecord dropPropertyRecord;
+        AddPropertyRecord addPropertyRecord;
     };
 
     bool operator==(const WALRecord& rhs) const {
@@ -446,6 +462,9 @@ struct WALRecord {
         case WALRecordType::DROP_PROPERTY_RECORD: {
             return dropPropertyRecord == rhs.dropPropertyRecord;
         }
+        case WALRecordType::ADD_PROPERTY_RECORD: {
+            return addPropertyRecord == rhs.addPropertyRecord;
+        }
         default: {
             throw RuntimeException("Unrecognized WAL record type inside ==. recordType: " +
                                    walRecordTypeToString(recordType));
@@ -468,6 +487,7 @@ struct WALRecord {
     static WALRecord newCopyRelRecord(table_id_t tableID);
     static WALRecord newDropTableRecord(table_id_t tableID);
     static WALRecord newDropPropertyRecord(table_id_t tableID, property_id_t propertyID);
+    static WALRecord newAddPropertyRecord(table_id_t tableID, property_id_t propertyID);
     static void constructWALRecordFromBytes(WALRecord& retVal, uint8_t* bytes, uint64_t& offset);
     // This functions assumes that the caller ensures there is enough space in the bytes pointer
     // to write the record. This should be checked by calling numBytesToWrite.

diff --git a/src/include/storage/wal_replayer_utils.h b/src/include/storage/wal_replayer_utils.h
@@ -46,6 +46,12 @@ class WALReplayerUtils {
             directory, tableID, propertyID, DBFileType::ORIGINAL));
     }
 
+    static inline void renameDBFilesForNodeProperty(
+        const string& directory, table_id_t tableID, property_id_t propertyID) {
+        replaceOriginalColumnFilesWithWALVersionIfExists(StorageUtils::getNodePropertyColumnFName(
+            directory, tableID, propertyID, DBFileType::ORIGINAL));
+    }
+
     static void removeDBFilesForRelProperty(
         const string& directory, RelTableSchema* relTableSchema, property_id_t propertyID);
 
@@ -55,6 +61,9 @@ class WALReplayerUtils {
     static void createEmptyDBFilesForNewNodeTable(
         NodeTableSchema* nodeTableSchema, const string& directory);
 
+    static void renameDBFilesForRelProperty(
+        const string& directory, RelTableSchema* relTableSchema, property_id_t propertyID);
+
 private:
     static inline void removeColumnFilesForPropertyIfExists(const string& directory,
         table_id_t relTableID, table_id_t boundTableID, RelDirection relDirection,

diff --git a/src/processor/operator/ddl/add_node_property.cpp b/src/processor/operator/ddl/add_node_property.cpp
@@ -11,7 +11,6 @@ void AddNodeProperty::executeDDLInternal() {
         property, getDefaultVal(), isDefaultValueNull(),
         storageManager.getNodesStore().getNodesStatisticsAndDeletedIDs().getNumTuplesForTable(
             tableID));
-    storageManager.getNodesStore().getNodeTable(tableID)->addProperty(property);
 }
 
 } // namespace processor

diff --git a/src/processor/operator/ddl/add_rel_property.cpp b/src/processor/operator/ddl/add_rel_property.cpp
@@ -6,11 +6,9 @@ namespace processor {
 void AddRelProperty::executeDDLInternal() {
     AddProperty::executeDDLInternal();
     auto tableSchema = catalog->getWriteVersion()->getRelTableSchema(tableID);
-    auto propertyID = tableSchema->getPropertyID(propertyName);
-    auto property = tableSchema->getProperty(propertyID);
+    auto property = tableSchema->getProperty(tableSchema->getPropertyID(propertyName));
     StorageUtils::createFileForRelPropertyWithDefaultVal(
         tableSchema, property, getDefaultVal(), isDefaultValueNull(), storageManager);
-    storageManager.getRelsStore().getRelTable(tableID)->addProperty(property, tableID);
 }
 
 } // namespace processor

diff --git a/src/storage/storage_utils.cpp b/src/storage/storage_utils.cpp
@@ -108,7 +108,7 @@ void StorageUtils::createFileForNodePropertyWithDefaultVal(table_id_t tableID,
     bool isDefaultValNull, uint64_t numNodes) {
     auto inMemColumn = InMemColumnFactory::getInMemPropertyColumn(
         StorageUtils::getNodePropertyColumnFName(
-            directory, tableID, property.propertyID, DBFileType::ORIGINAL),
+            directory, tableID, property.propertyID, DBFileType::WAL_VERSION),
         property.dataType, numNodes);
     if (!isDefaultValNull) {
         inMemColumn->fillWithDefaultVal(defaultVal, numNodes, property.dataType);
@@ -135,7 +135,7 @@ void StorageUtils::createFileForRelColumnPropertyWithDefaultVal(table_id_t relTa
     uint8_t* defaultVal, bool isDefaultValNull, StorageManager& storageManager) {
     auto inMemColumn = InMemColumnFactory::getInMemPropertyColumn(
         StorageUtils::getRelPropertyColumnFName(storageManager.getDirectory(), relTableID,
-            boundTableID, direction, property.propertyID, DBFileType::ORIGINAL),
+            boundTableID, direction, property.propertyID, DBFileType::WAL_VERSION),
         property.dataType,
         storageManager.getRelsStore().getRelsStatistics().getNumTuplesForTable(relTableID));
     if (!isDefaultValNull) {
@@ -152,7 +152,7 @@ void StorageUtils::createFileForRelListsPropertyWithDefaultVal(table_id_t relTab
     uint8_t* defaultVal, bool isDefaultValNull, StorageManager& storageManager) {
     auto inMemList = InMemListsFactory::getInMemPropertyLists(
         StorageUtils::getRelPropertyListsFName(storageManager.getDirectory(), relTableID,
-            boundTableID, direction, property.propertyID, DBFileType::ORIGINAL),
+            boundTableID, direction, property.propertyID, DBFileType::WAL_VERSION),
         property.dataType,
         storageManager.getRelsStore().getRelsStatistics().getNumTuplesForTable(relTableID));
     // Note: we need the listMetadata to get the num of elements in a large list, and headers to

diff --git a/src/storage/store/rel_table.cpp b/src/storage/store/rel_table.cpp
@@ -340,9 +340,9 @@ void RelTable::initEmptyRelsForNewNode(nodeID_t& nodeID) {
     listsUpdatesStore->initNewlyAddedNodes(nodeID);
 }
 
-void RelTable::addProperty(Property property, table_id_t tableID) {
-    fwdRelTableData->addProperty(property, tableID, wal);
-    bwdRelTableData->addProperty(property, tableID, wal);
+void RelTable::addProperty(Property property) {
+    fwdRelTableData->addProperty(property, wal);
+    bwdRelTableData->addProperty(property, wal);
 }
 
 void RelTable::appendInMemListToLargeListOP(
@@ -391,7 +391,7 @@ void DirectedRelTableData::removeProperty(property_id_t propertyID) {
     }
 }
 
-void DirectedRelTableData::addProperty(Property& property, table_id_t tableID, WAL* wal) {
+void DirectedRelTableData::addProperty(Property& property, WAL* wal) {
     for (auto& [boundTableID, propertyColumnsPerBoundTable] : propertyColumns) {
         propertyColumnsPerBoundTable.emplace(property.propertyID,
             ColumnFactory::getColumn(

diff --git a/src/storage/wal/wal.cpp b/src/storage/wal/wal.cpp
@@ -96,6 +96,12 @@ void WAL::logDropPropertyRecord(table_id_t tableID, property_id_t propertyID) {
     addNewWALRecordNoLock(walRecord);
 }
 
+void WAL::logAddPropertyRecord(table_id_t tableID, property_id_t propertyID) {
+    lock_t lck{mtx};
+    WALRecord walRecord = WALRecord::newAddPropertyRecord(tableID, propertyID);
+    addNewWALRecordNoLock(walRecord);
+}
+
 void WAL::clearWAL() {
     bufferManager.removeFilePagesFromFrames(*fileHandle);
     fileHandle->resetToZeroPagesAndPageCapacity();

diff --git a/src/storage/wal/wal_record.cpp b/src/storage/wal/wal_record.cpp
@@ -211,6 +211,13 @@ WALRecord WALRecord::newDropPropertyRecord(table_id_t tableID, property_id_t pro
     return retVal;
 }
 
+WALRecord WALRecord::newAddPropertyRecord(table_id_t tableID, property_id_t propertyID) {
+    WALRecord retVal;
+    retVal.recordType = WALRecordType::ADD_PROPERTY_RECORD;
+    retVal.addPropertyRecord = AddPropertyRecord(tableID, propertyID);
+    return retVal;
+}
+
 void WALRecord::constructWALRecordFromBytes(WALRecord& retVal, uint8_t* bytes, uint64_t& offset) {
     ((WALRecord*)&retVal)[0] = ((WALRecord*)(bytes + offset))[0];
     offset += sizeof(WALRecord);

diff --git a/src/storage/wal_replayer.cpp b/src/storage/wal_replayer.cpp
@@ -361,6 +361,39 @@ void WALReplayer::replayWALRecord(WALRecord& walRecord) {
             // See comments for COPY_NODE_RECORD.
         }
     } break;
+    case WALRecordType::ADD_PROPERTY_RECORD: {
+        if (isCheckpoint) {
+            auto tableID = walRecord.addPropertyRecord.tableID;
+            auto propertyID = walRecord.addPropertyRecord.propertyID;
+            if (!isRecovering) {
+                auto tableSchema = catalog->getWriteVersion()->getTableSchema(tableID);
+                auto property = tableSchema->getProperty(propertyID);
+                if (catalog->getReadOnlyVersion()->containNodeTable(tableID)) {
+                    WALReplayerUtils::renameDBFilesForNodeProperty(
+                        wal->getDirectory(), tableID, propertyID);
+                    storageManager->getNodesStore().getNodeTable(tableID)->addProperty(property);
+                } else {
+                    WALReplayerUtils::renameDBFilesForRelProperty(wal->getDirectory(),
+                        reinterpret_cast<RelTableSchema*>(tableSchema), propertyID);
+                    storageManager->getRelsStore().getRelTable(tableID)->addProperty(property);
+                }
+            } else {
+                auto catalogForRecovery = getCatalogForRecovery(DBFileType::WAL_VERSION);
+                auto tableSchema =
+                    catalogForRecovery->getReadOnlyVersion()->getTableSchema(tableID);
+                auto property = tableSchema->getProperty(propertyID);
+                if (catalogForRecovery->getReadOnlyVersion()->containNodeTable(tableID)) {
+                    WALReplayerUtils::renameDBFilesForNodeProperty(
+                        wal->getDirectory(), tableID, propertyID);
+                } else {
+                    WALReplayerUtils::renameDBFilesForRelProperty(wal->getDirectory(),
+                        reinterpret_cast<RelTableSchema*>(tableSchema), propertyID);
+                }
+            }
+        } else {
+            // See comments for COPY_NODE_RECORD.
+        }
+    } break;
     default:
         throw RuntimeException(
             "Unrecognized WAL record type inside WALReplayer::replay. recordType: " +

diff --git a/src/storage/wal_replayer_utils.cpp b/src/storage/wal_replayer_utils.cpp
@@ -62,6 +62,23 @@ void WALReplayerUtils::createEmptyDBFilesForNewNodeTable(
     }
 }
 
+void WALReplayerUtils::renameDBFilesForRelProperty(const std::string& directory,
+    kuzu::catalog::RelTableSchema* relTableSchema, kuzu::common::property_id_t propertyID) {
+    for (auto direction : REL_DIRECTIONS) {
+        for (auto boundTableID : relTableSchema->getUniqueBoundTableIDs(direction)) {
+            if (relTableSchema->isSingleMultiplicityInDirection(direction)) {
+                replaceOriginalColumnFilesWithWALVersionIfExists(
+                    StorageUtils::getRelPropertyColumnFName(directory, relTableSchema->tableID,
+                        boundTableID, direction, propertyID, DBFileType::ORIGINAL));
+            } else {
+                replaceOriginalListFilesWithWALVersionIfExists(
+                    StorageUtils::getRelPropertyListsFName(directory, relTableSchema->tableID,
+                        boundTableID, direction, propertyID, DBFileType::ORIGINAL));
+            }
+        }
+    }
+}
+
 void WALReplayerUtils::initLargeListPageListsAndSaveToFile(InMemLists* inMemLists) {
     inMemLists->getListsMetadataBuilder()->initLargeListPageLists(0 /* largeListIdx */);
     inMemLists->saveToFile();