Remove redundant computation of isNewNodeGroup

src/include/storage/store/column.h

@@ -92,12 +92,14 @@ class Column {
 
     virtual void prepareCommit();
     virtual void prepareCommitForChunk(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, const ChunkCollection& localInsertChunks,
-        const offset_to_row_idx_t& insertInfo, const ChunkCollection& localUpdateChunks,
-        const offset_to_row_idx_t& updateInfo, const offset_set_t& deleteInfo);
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup,
+        const ChunkCollection& localInsertChunks, const offset_to_row_idx_t& insertInfo,
+        const ChunkCollection& localUpdateChunks, const offset_to_row_idx_t& updateInfo,
+        const offset_set_t& deleteInfo);
     virtual void prepareCommitForChunk(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, const std::vector<common::offset_t>& dstOffsets,
-        ColumnChunk* chunk, common::offset_t startSrcOffset);
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup,
+        const std::vector<common::offset_t>& dstOffsets, ColumnChunk* chunk,
+        common::offset_t startSrcOffset);
 
     virtual void checkpointInMemory();
     virtual void rollbackInMemory();

src/include/storage/store/list_column.h

@@ -95,12 +95,14 @@ class ListColumn final : public Column {
         common::offset_t endOffsetInNodeGroup);
 
     void prepareCommitForChunk(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, const ChunkCollection& localInsertChunks,
-        const offset_to_row_idx_t& insertInfo, const ChunkCollection& localUpdateChunks,
-        const offset_to_row_idx_t& updateInfo, const offset_set_t& deleteInfo) override;
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup,
+        const ChunkCollection& localInsertChunks, const offset_to_row_idx_t& insertInfo,
+        const ChunkCollection& localUpdateChunks, const offset_to_row_idx_t& updateInfo,
+        const offset_set_t& deleteInfo) override;
     void prepareCommitForChunk(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, const std::vector<common::offset_t>& dstOffsets,
-        ColumnChunk* chunk, common::offset_t startSrcOffset) override;
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup,
+        const std::vector<common::offset_t>& dstOffsets, ColumnChunk* chunk,
+        common::offset_t startSrcOffset) override;
 
     void prepareCommitForOffsetChunk(transaction::Transaction* transaction, ChunkState& offsetState,
         const std::vector<common::offset_t>& dstOffsets, ColumnChunk* chunk,

src/include/storage/store/rel_table.h

@@ -73,7 +73,7 @@ class RelTable final : public Table {
         MemoryManager* memoryManager, catalog::RelTableCatalogEntry* relTableEntry, WAL* wal,
         bool enableCompression);
 
-    inline void initializeReadState(transaction::Transaction* transaction,
+    void initializeReadState(transaction::Transaction* transaction,
         common::RelDataDirection direction, const std::vector<common::column_id_t>& columnIDs,
         const common::ValueVector& inNodeIDVector, RelTableReadState& readState) {
         if (!readState.dataReadState) {
@@ -100,38 +100,38 @@ class RelTable final : public Table {
 
     void addColumn(transaction::Transaction* transaction, const catalog::Property& property,
         common::ValueVector* defaultValueVector) override;
-    inline void dropColumn(common::column_id_t columnID) override {
+    void dropColumn(common::column_id_t columnID) override {
         fwdRelTableData->dropColumn(columnID);
         bwdRelTableData->dropColumn(columnID);
     }
-    inline Column* getCSROffsetColumn(common::RelDataDirection direction) {
+    Column* getCSROffsetColumn(common::RelDataDirection direction) {
         return direction == common::RelDataDirection::FWD ? fwdRelTableData->getCSROffsetColumn() :
                                                             bwdRelTableData->getCSROffsetColumn();
     }
-    inline Column* getCSRLengthColumn(common::RelDataDirection direction) {
+    Column* getCSRLengthColumn(common::RelDataDirection direction) {
         return direction == common::RelDataDirection::FWD ? fwdRelTableData->getCSRLengthColumn() :
                                                             bwdRelTableData->getCSRLengthColumn();
     }
-    inline common::column_id_t getNumColumns() {
+    common::column_id_t getNumColumns() {
         KU_ASSERT(fwdRelTableData->getNumColumns() == bwdRelTableData->getNumColumns());
         return fwdRelTableData->getNumColumns();
     }
-    inline Column* getColumn(common::column_id_t columnID, common::RelDataDirection direction) {
+    Column* getColumn(common::column_id_t columnID, common::RelDataDirection direction) {
         return direction == common::RelDataDirection::FWD ? fwdRelTableData->getColumn(columnID) :
                                                             bwdRelTableData->getColumn(columnID);
     }
-    inline const std::vector<std::unique_ptr<Column>>& getColumns(
+    const std::vector<std::unique_ptr<Column>>& getColumns(
         common::RelDataDirection direction) const {
         return direction == common::RelDataDirection::FWD ? fwdRelTableData->getColumns() :
                                                             bwdRelTableData->getColumns();
     }
 
-    inline void append(ChunkedNodeGroup* nodeGroup, common::RelDataDirection direction) {
+    void append(ChunkedNodeGroup* nodeGroup, common::RelDataDirection direction) {
         direction == common::RelDataDirection::FWD ? fwdRelTableData->append(nodeGroup) :
                                                      bwdRelTableData->append(nodeGroup);
     }
 
-    inline bool isNewNodeGroup(transaction::Transaction* transaction,
+    bool isNewNodeGroup(transaction::Transaction* transaction,
         common::node_group_idx_t nodeGroupIdx, common::RelDataDirection direction) {
         return direction == common::RelDataDirection::FWD ?
                    fwdRelTableData->isNewNodeGroup(transaction, nodeGroupIdx) :
@@ -144,7 +144,7 @@ class RelTable final : public Table {
     void checkpointInMemory() override;
     void rollbackInMemory() override;
 
-    inline RelTableData* getDirectedTableData(common::RelDataDirection direction) {
+    RelTableData* getDirectedTableData(common::RelDataDirection direction) {
         return direction == common::RelDataDirection::FWD ? fwdRelTableData.get() :
                                                             bwdRelTableData.get();
     }

src/include/storage/store/rel_table_data.h

@@ -193,21 +193,21 @@ class RelTableData final : public TableData {
     double getHighDensity(uint64_t level) const;
 
     void updateCSRHeader(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, PersistentState& persistentState,
-        LocalState& localState);
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup,
+        PersistentState& persistentState, LocalState& localState);
     void commitCSRHeaderChunk(transaction::Transaction* transaction, bool isNewNodeGroup,
         common::node_group_idx_t nodeGroupIdx, Column* column, ColumnChunk* columnChunk,
         LocalState& localState, const std::vector<common::offset_t>& dstOffsets);
 
     void distributeOffsets(const ChunkedCSRHeader& header, LocalState& localState,
         common::offset_t leftBoundary, common::offset_t rightBoundary);
     void updateRegion(transaction::Transaction* transaction, common::node_group_idx_t nodeGroupIdx,
-        PersistentState& persistentState, LocalState& localState);
+        bool isNewNodeGroup, PersistentState& persistentState, LocalState& localState);
     void updateColumn(transaction::Transaction* transaction, common::node_group_idx_t nodeGroupIdx,
-        common::column_id_t columnID, const RelTableData::PersistentState& persistentState,
-        LocalState& localState);
+        bool isNewNodeGroup, common::column_id_t columnID,
+        const RelTableData::PersistentState& persistentState, LocalState& localState);
     void distributeAndUpdateColumn(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, common::column_id_t columnID,
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup, common::column_id_t columnID,
         const PersistentState& persistentState, LocalState& localState);
 
     void findPositionsForInsertions(common::offset_t nodeOffset, common::length_t numInsertions,
@@ -227,16 +227,17 @@ class RelTableData final : public TableData {
         ColumnChunk* chunk);
 
     void applyUpdatesToColumn(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, common::column_id_t columnID,
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup, common::column_id_t columnID,
         const PersistentState& persistentState, LocalState& localState, Column* column);
     void applyInsertionsToColumn(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, common::column_id_t columnID, LocalState& localState,
-        const PersistentState& persistentState, Column* column);
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup, common::column_id_t columnID,
+        LocalState& localState, const PersistentState& persistentState, Column* column);
     void applyDeletionsToColumn(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, LocalState& localState,
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup, LocalState& localState,
         const PersistentState& persistentState, Column* column);
     void applySliding(transaction::Transaction* transaction, common::node_group_idx_t nodeGroupIdx,
-        LocalState& localState, const PersistentState& persistentState, Column* column);
+        bool isNewNodeGroup, LocalState& localState, const PersistentState& persistentState,
+        Column* column);
 
     std::vector<std::pair<common::offset_t, common::offset_t>> getSlidesForDeletions(
         const PersistentState& persistentState, const LocalState& localState);

src/include/storage/store/struct_column.h

@@ -37,12 +37,14 @@ class StructColumn final : public Column {
         common::offset_t dataOffset, common::length_t numValues) override;
 
     void prepareCommitForChunk(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, const ChunkCollection& localInsertChunk,
-        const offset_to_row_idx_t& insertInfo, const ChunkCollection& localUpdateChunk,
-        const offset_to_row_idx_t& updateInfo, const offset_set_t& deleteInfo) override;
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup,
+        const ChunkCollection& localInsertChunk, const offset_to_row_idx_t& insertInfo,
+        const ChunkCollection& localUpdateChunk, const offset_to_row_idx_t& updateInfo,
+        const offset_set_t& deleteInfo) override;
     void prepareCommitForChunk(transaction::Transaction* transaction,
-        common::node_group_idx_t nodeGroupIdx, const std::vector<common::offset_t>& dstOffsets,
-        ColumnChunk* chunk, common::offset_t startSrcOffset) override;
+        common::node_group_idx_t nodeGroupIdx, bool isNewNodeGroup,
+        const std::vector<common::offset_t>& dstOffsets, ColumnChunk* chunk,
+        common::offset_t startSrcOffset) override;
 
 protected:
     void scanInternal(transaction::Transaction* transaction, ChunkState& readState,

src/storage/store/column.cpp

@@ -601,11 +601,9 @@ void Column::prepareCommit() {
 }
 
 void Column::prepareCommitForChunk(Transaction* transaction, common::node_group_idx_t nodeGroupIdx,
-    const ChunkCollection& localInsertChunks, const offset_to_row_idx_t& insertInfo,
-    const ChunkCollection& localUpdateChunks, const offset_to_row_idx_t& updateInfo,
-    const offset_set_t& deleteInfo) {
-    auto currentNumNodeGroups = metadataDA->getNumElements(transaction->getType());
-    auto isNewNodeGroup = nodeGroupIdx >= currentNumNodeGroups;
+    bool isNewNodeGroup, const ChunkCollection& localInsertChunks,
+    const offset_to_row_idx_t& insertInfo, const ChunkCollection& localUpdateChunks,
+    const offset_to_row_idx_t& updateInfo, const offset_set_t& deleteInfo) {
     if (isNewNodeGroup) {
         // If this is a new node group, updateInfo should be empty. We should perform out-of-place
         // commit with a new column chunk.
@@ -652,11 +650,9 @@ void Column::prepareCommitForChunk(Transaction* transaction, common::node_group_
 }
 
 void Column::prepareCommitForChunk(Transaction* transaction, node_group_idx_t nodeGroupIdx,
-    const std::vector<common::offset_t>& dstOffsets, ColumnChunk* chunk, offset_t startSrcOffset) {
+    bool isNewNodeGroup, const std::vector<common::offset_t>& dstOffsets, ColumnChunk* chunk,
+    offset_t startSrcOffset) {
     metadataDA->prepareCommit();
-    auto currentNumNodeGroups = metadataDA->getNumElements(transaction->getType());
-    // TODO: Move newNodeGroup out to the table data level.
-    auto isNewNodeGroup = nodeGroupIdx >= currentNumNodeGroups;
     if (isNewNodeGroup) {
         commitColumnChunkOutOfPlace(transaction, nodeGroupIdx, isNewNodeGroup, dstOffsets, chunk,
             startSrcOffset);
@@ -822,8 +818,6 @@ void Column::commitColumnChunkOutOfPlace(Transaction* transaction, node_group_id
         append(chunk, nodeGroupIdx);
     } else {
         auto chunkMeta = getMetadata(nodeGroupIdx, transaction->getType());
-        // TODO(Guodong): Should consider caching the scanned column chunk to avoid redundant
-        // scans in the same transaction.
         auto columnChunk =
             getEmptyChunkForCommit(1.5 * std::bit_ceil(chunkMeta.numValues + dstOffsets.size()));
         scan(transaction, nodeGroupIdx, columnChunk.get());

src/storage/store/list_column.cpp

@@ -4,6 +4,7 @@
 #include "storage/store/list_column_chunk.h"
 #include "storage/store/null_column.h"
 #include <bit>
+
 using namespace kuzu::common;
 using namespace kuzu::transaction;
 
@@ -322,11 +323,9 @@ ListOffsetSizeInfo ListColumn::getListOffsetSizeInfo(Transaction* transaction,
 }
 
 void ListColumn::prepareCommitForChunk(Transaction* transaction, node_group_idx_t nodeGroupIdx,
-    const ChunkCollection& localInsertChunks, const offset_to_row_idx_t& insertInfo,
-    const ChunkCollection& localUpdateChunks, const offset_to_row_idx_t& updateInfo,
-    const offset_set_t& deleteInfo) {
-    auto currentNumNodeGroups = metadataDA->getNumElements(transaction->getType());
-    auto isNewNodeGroup = nodeGroupIdx >= currentNumNodeGroups;
+    bool isNewNodeGroup, const ChunkCollection& localInsertChunks,
+    const offset_to_row_idx_t& insertInfo, const ChunkCollection& localUpdateChunks,
+    const offset_to_row_idx_t& updateInfo, const offset_set_t& deleteInfo) {
     if (isNewNodeGroup) {
         commitLocalChunkOutOfPlace(transaction, nodeGroupIdx, isNewNodeGroup, localInsertChunks,
             insertInfo, localUpdateChunks, updateInfo, deleteInfo);
@@ -347,15 +346,14 @@ void ListColumn::prepareCommitForChunk(Transaction* transaction, node_group_idx_
             dstOffsets.push_back(offsetInDstChunk);
             columnChunk->append(localInsertChunk, offsetInLocalChunk, 1);
         }
-        prepareCommitForChunk(transaction, nodeGroupIdx, dstOffsets, columnChunk.get(),
-            0 /*startSrcOffset*/);
+        prepareCommitForChunk(transaction, nodeGroupIdx, isNewNodeGroup, dstOffsets,
+            columnChunk.get(), 0 /*startSrcOffset*/);
     }
 }
 
 void ListColumn::prepareCommitForChunk(Transaction* transaction, node_group_idx_t nodeGroupIdx,
-    const std::vector<common::offset_t>& dstOffsets, ColumnChunk* chunk, offset_t startSrcOffset) {
-    auto currentNumNodeGroups = metadataDA->getNumElements(transaction->getType());
-    auto isNewNodeGroup = nodeGroupIdx >= currentNumNodeGroups;
+    bool isNewNodeGroup, const std::vector<common::offset_t>& dstOffsets, ColumnChunk* chunk,
+    offset_t startSrcOffset) {
     if (isNewNodeGroup) {
         commitColumnChunkOutOfPlace(transaction, nodeGroupIdx, isNewNodeGroup, dstOffsets, chunk,
             startSrcOffset);
@@ -389,7 +387,7 @@ void ListColumn::prepareCommitForChunk(Transaction* transaction, node_group_idx_
         // TODO: Shouldn't here be in place commit?
         dataColumn->commitColumnChunkOutOfPlace(transaction, nodeGroupIdx, isNewNodeGroup,
             dstOffsetsInDataColumn, dataColumnChunk, startListOffset);
-        sizeColumn->prepareCommitForChunk(transaction, nodeGroupIdx, dstOffsets,
+        sizeColumn->prepareCommitForChunk(transaction, nodeGroupIdx, isNewNodeGroup, dstOffsets,
             listChunk->getSizeColumnChunk(), startSrcOffset);
         for (auto i = 0u; i < numListsToAppend; i++) {
             auto listEndOffset = listChunk->getListEndOffset(startSrcOffset + i);

src/storage/store/node_table_data.cpp

@@ -191,14 +191,19 @@ void NodeTableData::append(ChunkedNodeGroup* nodeGroup) {
 
 void NodeTableData::prepareLocalNodeGroupToCommit(node_group_idx_t nodeGroupIdx,
     Transaction* transaction, LocalNodeNG* localNodeGroup) {
+    auto numNodeGroups = columns[0]->getNumNodeGroups(transaction);
+    auto isNewNodeGroup = nodeGroupIdx >= numNodeGroups;
+    KU_ASSERT(std::find_if(columns.begin(), columns.end(), [&](const auto& column) {
+        return column->getNumNodeGroups(transaction) != numNodeGroups;
+    }) == columns.end());
     for (auto columnID = 0u; columnID < columns.size(); columnID++) {
         auto column = columns[columnID].get();
         auto localInsertChunk = localNodeGroup->getInsertChunks().getLocalChunk(columnID);
         auto localUpdateChunk = localNodeGroup->getUpdateChunks(columnID).getLocalChunk(0);
         if (localInsertChunk.empty() && localUpdateChunk.empty()) {
             continue;
         }
-        column->prepareCommitForChunk(transaction, nodeGroupIdx, localInsertChunk,
+        column->prepareCommitForChunk(transaction, nodeGroupIdx, isNewNodeGroup, localInsertChunk,
             localNodeGroup->getInsertInfoRef(), localUpdateChunk,
             localNodeGroup->getUpdateInfoRef(columnID), {} /* deleteInfo */);
     }