Merge pull request #1175 from kuzudb/drop-prop-trx

add transaction to drop-property

src/binder/bind/bind_ddl.cpp

@@ -65,9 +65,14 @@ unique_ptr<BoundStatement> Binder::bindDropProperty(const Statement& statement)
     auto isNodeTable = catalogContent->containNodeTable(tableName);
     auto tableID = isNodeTable ? catalogContent->getNodeTableIDFromName(tableName) :
                                  catalogContent->getRelTableIDFromName(tableName);
-    return make_unique<BoundDropProperty>(tableID,
-        bindPropertyName(catalogContent->getTableSchema(tableID), dropProperty.getPropertyName()),
-        tableName);
+    auto propertyID =
+        bindPropertyName(catalogContent->getTableSchema(tableID), dropProperty.getPropertyName());
+    if (isNodeTable &&
+        ((NodeTableSchema*)catalogContent->getTableSchema(tableID))->primaryKeyPropertyID ==
+            propertyID) {
+        throw BinderException("Cannot drop primary key of a node table.");
+    }
+    return make_unique<BoundDropProperty>(tableID, propertyID, tableName);
 }
 
 vector<PropertyNameDataType> Binder::bindPropertyNameDataTypes(
@@ -113,7 +118,7 @@ uint32_t Binder::bindPrimaryKey(
     return primaryKeyIdx;
 }
 
-property_id_t Binder::bindPropertyName(TableSchema* tableSchema, string propertyName) {
+property_id_t Binder::bindPropertyName(TableSchema* tableSchema, const string& propertyName) {
     for (auto& property : tableSchema->properties) {
         if (property.name == propertyName) {
             return property.propertyID;

src/catalog/catalog.cpp

@@ -128,7 +128,7 @@ uint64_t SerDeser::serializeValue<NodeTableSchema>(
     const NodeTableSchema& value, FileInfo* fileInfo, uint64_t offset) {
     offset = SerDeser::serializeValue<string>(value.tableName, fileInfo, offset);
     offset = SerDeser::serializeValue<table_id_t>(value.tableID, fileInfo, offset);
-    offset = SerDeser::serializeValue<property_id_t>(value.primaryKeyPropertyIdx, fileInfo, offset);
+    offset = SerDeser::serializeValue<property_id_t>(value.primaryKeyPropertyID, fileInfo, offset);
     offset = SerDeser::serializeVector<Property>(value.properties, fileInfo, offset);
     offset = SerDeser::serializeUnorderedSet<table_id_t>(value.fwdRelTableIDSet, fileInfo, offset);
     return SerDeser::serializeUnorderedSet<table_id_t>(value.bwdRelTableIDSet, fileInfo, offset);
@@ -140,7 +140,7 @@ uint64_t SerDeser::deserializeValue<NodeTableSchema>(
     offset = SerDeser::deserializeValue<string>(value.tableName, fileInfo, offset);
     offset = SerDeser::deserializeValue<table_id_t>(value.tableID, fileInfo, offset);
     offset =
-        SerDeser::deserializeValue<property_id_t>(value.primaryKeyPropertyIdx, fileInfo, offset);
+        SerDeser::deserializeValue<property_id_t>(value.primaryKeyPropertyID, fileInfo, offset);
     offset = SerDeser::deserializeVector<Property>(value.properties, fileInfo, offset);
     offset =
         SerDeser::deserializeUnorderedSet<table_id_t>(value.fwdRelTableIDSet, fileInfo, offset);
@@ -263,6 +263,17 @@ vector<Property> CatalogContent::getAllNodeProperties(table_id_t tableID) const
     return nodeTableSchemas.at(tableID)->getAllNodeProperties();
 }
 
+void CatalogContent::dropTableSchema(table_id_t tableID) {
+    auto tableSchema = getTableSchema(tableID);
+    if (tableSchema->isNodeTable) {
+        nodeTableNameToIDMap.erase(tableSchema->tableName);
+        nodeTableSchemas.erase(tableID);
+    } else {
+        relTableNameToIDMap.erase(tableSchema->tableName);
+        relTableSchemas.erase(tableID);
+    }
+}
+
 void CatalogContent::saveToFile(const string& directory, DBFileType dbFileType) {
     auto catalogPath = StorageUtils::getCatalogFilePath(directory, dbFileType);
     auto fileInfo = FileUtils::openFile(catalogPath, O_WRONLY | O_CREAT);
@@ -313,17 +324,6 @@ void CatalogContent::readFromFile(const string& directory, DBFileType dbFileType
     SerDeser::deserializeValue<table_id_t>(nextTableID, fileInfo.get(), offset);
 }
 
-void CatalogContent::removeTableSchema(TableSchema* tableSchema) {
-    auto tableID = tableSchema->tableID;
-    if (tableSchema->isNodeTable) {
-        nodeTableNameToIDMap.erase(tableSchema->tableName);
-        nodeTableSchemas.erase(tableID);
-    } else {
-        relTableNameToIDMap.erase(tableSchema->tableName);
-        relTableSchemas.erase(tableID);
-    }
-}
-
 Catalog::Catalog() : wal{nullptr} {
     catalogContentForReadOnlyTrx = make_unique<CatalogContent>();
     builtInVectorOperations = make_unique<BuiltInVectorOperations>();
@@ -372,5 +372,17 @@ table_id_t Catalog::addRelTableSchema(string tableName, RelMultiplicity relMulti
     return tableID;
 }
 
+void Catalog::dropTableSchema(table_id_t tableID) {
+    initCatalogContentForWriteTrxIfNecessary();
+    catalogContentForWriteTrx->dropTableSchema(tableID);
+    wal->logDropTableRecord(tableID);
+}
+
+void Catalog::dropProperty(table_id_t tableID, property_id_t propertyID) {
+    initCatalogContentForWriteTrxIfNecessary();
+    catalogContentForWriteTrx->dropProperty(tableID, propertyID);
+    wal->logDropPropertyRecord(tableID, propertyID);
+}
+
 } // namespace catalog
 } // namespace kuzu

src/include/binder/binder.h

@@ -49,7 +49,7 @@ class Binder {
     vector<PropertyNameDataType> bindPropertyNameDataTypes(
         vector<pair<string, string>> propertyNameDataTypes);
     uint32_t bindPrimaryKey(string pkColName, vector<pair<string, string>> propertyNameDataTypes);
-    property_id_t bindPropertyName(TableSchema* tableSchema, string propertyName);
+    property_id_t bindPropertyName(TableSchema* tableSchema, const string& propertyName);
 
     /*** bind copy csv ***/
     unique_ptr<BoundStatement> bindCopyCSV(const Statement& statement);

src/include/catalog/catalog.h

@@ -126,8 +126,11 @@ class CatalogContent {
         return direction == FWD ? relTableSchema->getUniqueSrcTableIDs() :
                                   relTableSchema->getUniqueDstTableIDs();
     }
+    inline void dropProperty(table_id_t tableID, property_id_t propertyID) {
+        getTableSchema(tableID)->dropProperty(propertyID);
+    }
 
-    void removeTableSchema(TableSchema* tableSchema);
+    void dropTableSchema(table_id_t tableID);
 
     void saveToFile(const string& directory, DBFileType dbFileType);
     void readFromFile(const string& directory, DBFileType dbFileType);
@@ -148,7 +151,6 @@ class CatalogContent {
 
 class Catalog {
 public:
-    // This constructor is only used for mock catalog testing only.
     Catalog();
 
     explicit Catalog(WAL* wal);
@@ -193,11 +195,9 @@ class Catalog {
         const vector<PropertyNameDataType>& propertyDefinitions,
         vector<pair<table_id_t, table_id_t>> srcDstTableIDs);
 
-    inline void removeTableSchema(TableSchema* tableSchema) {
-        initCatalogContentForWriteTrxIfNecessary();
-        catalogContentForWriteTrx->removeTableSchema(tableSchema);
-        wal->logDropTableRecord(tableSchema->isNodeTable, tableSchema->tableID);
-    }
+    void dropTableSchema(table_id_t tableID);
+
+    void dropProperty(table_id_t tableID, property_id_t propertyID);
 
 protected:
     unique_ptr<BuiltInVectorOperations> builtInVectorOperations;

src/include/catalog/catalog_structs.h

@@ -70,7 +70,7 @@ struct TableSchema {
 
     inline uint32_t getNumProperties() const { return properties.size(); }
 
-    inline void removeProperty(property_id_t propertyID) {
+    inline void dropProperty(property_id_t propertyID) {
         properties.erase(std::remove_if(properties.begin(), properties.end(),
                              [propertyID](const Property& property) {
                                  return property.propertyID == propertyID;
@@ -98,20 +98,20 @@ struct NodeTableSchema : TableSchema {
     NodeTableSchema(string tableName, table_id_t tableID, property_id_t primaryPropertyId,
         vector<Property> properties)
         : TableSchema{std::move(tableName), tableID, true /* isNodeTable */, std::move(properties)},
-          primaryKeyPropertyIdx{primaryPropertyId} {}
+          primaryKeyPropertyID{primaryPropertyId} {}
 
     inline void addFwdRelTableID(table_id_t tableID) { fwdRelTableIDSet.insert(tableID); }
     inline void addBwdRelTableID(table_id_t tableID) { bwdRelTableIDSet.insert(tableID); }
 
-    inline Property getPrimaryKey() const { return properties[primaryKeyPropertyIdx]; }
+    inline Property getPrimaryKey() const { return properties[primaryKeyPropertyID]; }
 
     inline vector<Property> getAllNodeProperties() const { return properties; }
 
     // TODO(Semih): When we support updating the schemas, we need to update this or, we need
     // a more robust mechanism to keep track of which property is the primary key (e.g., store this
     // information with the property). This is an idx, not an ID, so as the columns/properties of
     // the table change, the idx can change.
-    property_id_t primaryKeyPropertyIdx;
+    property_id_t primaryKeyPropertyID;
     unordered_set<table_id_t> fwdRelTableIDSet; // srcNode->rel
     unordered_set<table_id_t> bwdRelTableIDSet; // dstNode->rel
 };
@@ -162,6 +162,10 @@ struct RelTableSchema : TableSchema {
             });
     }
 
+    inline unordered_set<table_id_t> getUniqueBoundTableIDs(RelDirection relDirection) const {
+        return relDirection == FWD ? getUniqueSrcTableIDs() : getUniqueDstTableIDs();
+    }
+
     unordered_set<table_id_t> getAllNodeTableIDs() const;
 
     unordered_set<table_id_t> getUniqueSrcTableIDs() const;

src/include/processor/operator/ddl/drop_property.h

@@ -11,24 +11,21 @@ namespace processor {
 class DropProperty : public DDL {
 public:
     DropProperty(Catalog* catalog, table_id_t tableID, property_id_t propertyID,
-        StorageManager& storageManager, const DataPos& outputPos, uint32_t id,
-        const string& paramsString)
+        const DataPos& outputPos, uint32_t id, const string& paramsString)
         : DDL{PhysicalOperatorType::DROP_PROPERTY, catalog, outputPos, id, paramsString},
-          tableID{tableID}, propertyID{propertyID}, storageManager{storageManager} {}
+          tableID{tableID}, propertyID{propertyID} {}
 
-    void executeDDLInternal() override;
+    void executeDDLInternal() override { catalog->dropProperty(tableID, propertyID); }
 
-    std::string getOutputMsg() override;
+    string getOutputMsg() override { return {"Drop succeed."}; }
 
     unique_ptr<PhysicalOperator> clone() override {
-        return make_unique<DropProperty>(
-            catalog, tableID, propertyID, storageManager, outputPos, id, paramsString);
+        return make_unique<DropProperty>(catalog, tableID, propertyID, outputPos, id, paramsString);
     }
 
 protected:
     table_id_t tableID;
     property_id_t propertyID;
-    StorageManager& storageManager;
 };
 
 } // namespace processor

src/include/processor/operator/ddl/drop_table.h

@@ -10,23 +10,21 @@ namespace processor {
 
 class DropTable : public DDL {
 public:
-    DropTable(Catalog* catalog, table_id_t tableID, StorageManager& storageManager,
-        const DataPos& outputPos, uint32_t id, const string& paramsString)
+    DropTable(Catalog* catalog, table_id_t tableID, const DataPos& outputPos, uint32_t id,
+        const string& paramsString)
         : DDL{PhysicalOperatorType::DROP_TABLE, catalog, outputPos, id, paramsString},
-          tableID{tableID}, storageManager{storageManager} {}
+          tableID{tableID} {}
 
     void executeDDLInternal() override;
 
     std::string getOutputMsg() override;
 
     unique_ptr<PhysicalOperator> clone() override {
-        return make_unique<DropTable>(
-            catalog, tableID, storageManager, outputPos, id, paramsString);
+        return make_unique<DropTable>(catalog, tableID, outputPos, id, paramsString);
     }
 
 protected:
     table_id_t tableID;
-    StorageManager& storageManager;
 };
 
 } // namespace processor

src/include/storage/store/node_table.h

@@ -40,6 +40,7 @@ class NodeTable {
     inline table_id_t getTableID() const { return tableID; }
     inline void checkpointInMemoryIfNecessary() { pkIndex->checkpointInMemoryIfNecessary(); }
     inline void rollbackInMemoryIfNecessary() { pkIndex->rollbackInMemoryIfNecessary(); }
+    inline void removeProperty(property_id_t propertyID) { propertyColumns.erase(propertyID); }
 
     node_offset_t addNodeAndResetProperties(ValueVector* primaryKeyVector);
     void deleteNodes(ValueVector* nodeIDVector, ValueVector* primaryKeyVector);

src/include/storage/store/rel_table.h

@@ -126,6 +126,7 @@ class DirectedRelTableData {
     void performOpOnListsWithUpdates(const std::function<void(Lists*)>& opOnListsWithUpdates);
     unique_ptr<ListsUpdateIteratorsForDirection> getListsUpdateIteratorsForDirection(
         table_id_t boundNodeTableID);
+    void removeProperty(property_id_t propertyID);
 
 private:
     void scanColumns(Transaction* transaction, RelTableScanState& scanState,
@@ -189,6 +190,10 @@ class RelTable {
     inline DirectedRelTableData* getDirectedTableData(RelDirection relDirection) {
         return relDirection == FWD ? fwdRelTableData.get() : bwdRelTableData.get();
     }
+    inline void removeProperty(property_id_t propertyID) {
+        fwdRelTableData->removeProperty(propertyID);
+        bwdRelTableData->removeProperty(propertyID);
+    }
 
     vector<AdjLists*> getAdjListsForNodeTable(table_id_t tableID);
     vector<AdjColumn*> getAdjColumnsForNodeTable(table_id_t tableID);

src/include/storage/wal/wal.h

@@ -114,7 +114,9 @@ class WAL : public BaseWALAndWALIterator {
 
     void logCopyRelCSVRecord(table_id_t tableID);
 
-    void logDropTableRecord(bool isNodeTable, table_id_t tableID);
+    void logDropTableRecord(table_id_t tableID);
+
+    void logDropPropertyRecord(table_id_t tableID, property_id_t propertyID);
 
     // Removes the contents of WAL file.
     void clearWAL();