Use transactions to determine the read/write data to use

src/include/storage/store/property_statistics.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "common/types/types.h"
+#include "transaction/transaction.h"
 
 namespace kuzu {
 namespace storage {
@@ -34,8 +35,8 @@ class RWPropertyStats {
 
     static RWPropertyStats empty() { return RWPropertyStats(nullptr, 0, 0); }
 
-    bool mayHaveNull();
-    void setHasNull();
+    bool mayHaveNull(const transaction::Transaction& transaction);
+    void setHasNull(const transaction::Transaction& transaction);
 
 private:
     TablesStatistics* tablesStatistics;

src/include/storage/store/table_statistics.h

@@ -111,27 +111,27 @@ class TablesStatistics {
             ->getNumTuples();
     }
 
-    PropertyStatistics& getWritePropertyStatisticsForTable(
-        common::table_id_t tableID, common::property_id_t propertyID) {
-        initTableStatisticPerTableForWriteTrxIfNecessary();
-        auto tableStatistics =
-            tablesStatisticsContentForWriteTrx->tableStatisticPerTable.at(tableID).get();
-        return tableStatistics->getPropertyStatistics(propertyID);
-    }
-
-    inline PropertyStatistics* getReadPropertyStatisticsForTable(
-        common::table_id_t tableID, common::property_id_t propertyID) {
-        auto readResult =
-            tablesStatisticsContentForReadOnlyTrx->tableStatisticPerTable.find(tableID);
-        if (readResult != tablesStatisticsContentForReadOnlyTrx->tableStatisticPerTable.end()) {
-            return &(readResult->second->getPropertyStatistics(propertyID));
+    inline PropertyStatistics& getPropertyStatisticsForTable(
+        const transaction::Transaction& transaction, common::table_id_t tableID,
+        common::property_id_t propertyID) {
+        if (transaction.isReadOnly()) {
+            assert(tablesStatisticsContentForReadOnlyTrx->tableStatisticPerTable.contains(tableID));
+            auto tableStatistics =
+                tablesStatisticsContentForReadOnlyTrx->tableStatisticPerTable.at(tableID).get();
+            return tableStatistics->getPropertyStatistics(propertyID);
+        } else {
+            initTableStatisticPerTableForWriteTrxIfNecessary();
+            assert(tablesStatisticsContentForWriteTrx->tableStatisticPerTable.contains(tableID));
+            auto tableStatistics =
+                tablesStatisticsContentForWriteTrx->tableStatisticPerTable.at(tableID).get();
+            return tableStatistics->getPropertyStatistics(propertyID);
         }
-        return nullptr;
     }
 
     void setPropertyStatisticsForTable(
         common::table_id_t tableID, common::property_id_t propertyID, PropertyStatistics stats) {
         initTableStatisticPerTableForWriteTrxIfNecessary();
+        assert(tablesStatisticsContentForWriteTrx->tableStatisticPerTable.contains(tableID));
         auto tableStatistics =
             tablesStatisticsContentForWriteTrx->tableStatisticPerTable.at(tableID).get();
         tableStatistics->setPropertyStatistics(propertyID, stats);

src/include/transaction/transaction.h

@@ -47,6 +47,7 @@ class Transaction {
 };
 
 static Transaction DUMMY_READ_TRANSACTION = Transaction(TransactionType::READ_ONLY);
+static Transaction DUMMY_WRITE_TRANSACTION = Transaction(TransactionType::WRITE);
 
 } // namespace transaction
 } // namespace kuzu

src/storage/store/node_column.cpp

@@ -6,6 +6,7 @@
 #include "storage/store/struct_node_column.h"
 #include "storage/store/table_statistics.h"
 #include "storage/store/var_list_node_column.h"
+#include "transaction/transaction.h"
 
 using namespace kuzu::catalog;
 using namespace kuzu::common;
@@ -438,7 +439,7 @@ NullNodeColumn::NullNodeColumn(page_idx_t metaDAHPageIdx, BMFileHandle* dataFH,
 
 void NullNodeColumn::scan(
     Transaction* transaction, ValueVector* nodeIDVector, ValueVector* resultVector) {
-    if (propertyStatistics.mayHaveNull()) {
+    if (propertyStatistics.mayHaveNull(*transaction)) {
         scanInternal(transaction, nodeIDVector, resultVector);
     } else {
         resultVector->setAllNonNull();
@@ -448,7 +449,7 @@ void NullNodeColumn::scan(
 void NullNodeColumn::scan(transaction::Transaction* transaction, node_group_idx_t nodeGroupIdx,
     offset_t startOffsetInGroup, offset_t endOffsetInGroup, ValueVector* resultVector,
     uint64_t offsetInVector) {
-    if (propertyStatistics.mayHaveNull()) {
+    if (propertyStatistics.mayHaveNull(*transaction)) {
         NodeColumn::scan(transaction, nodeGroupIdx, startOffsetInGroup, endOffsetInGroup,
             resultVector, offsetInVector);
     } else {
@@ -457,7 +458,7 @@ void NullNodeColumn::scan(transaction::Transaction* transaction, node_group_idx_
 }
 
 void NullNodeColumn::scan(node_group_idx_t nodeGroupIdx, ColumnChunk* columnChunk) {
-    if (propertyStatistics.mayHaveNull()) {
+    if (propertyStatistics.mayHaveNull(DUMMY_WRITE_TRANSACTION)) {
         NodeColumn::scan(nodeGroupIdx, columnChunk);
     } else {
         static_cast<NullColumnChunk*>(columnChunk)->resetNullBuffer();
@@ -466,7 +467,7 @@ void NullNodeColumn::scan(node_group_idx_t nodeGroupIdx, ColumnChunk* columnChun
 
 void NullNodeColumn::lookup(
     Transaction* transaction, ValueVector* nodeIDVector, ValueVector* resultVector) {
-    if (propertyStatistics.mayHaveNull()) {
+    if (propertyStatistics.mayHaveNull(*transaction)) {
         lookupInternal(transaction, nodeIDVector, resultVector);
     } else {
         for (auto i = 0ul; i < nodeIDVector->state->selVector->selectedSize; i++) {
@@ -483,15 +484,15 @@ page_idx_t NullNodeColumn::append(
     metadataDA->update(nodeGroupIdx,
         MainColumnChunkMetadata{startPageIdx, numPagesFlushed, columnChunk->getNumValues()});
     if (static_cast<NullColumnChunk*>(columnChunk)->mayHaveNull()) {
-        propertyStatistics.setHasNull();
+        propertyStatistics.setHasNull(DUMMY_WRITE_TRANSACTION);
     }
     return numPagesFlushed;
 }
 
 void NullNodeColumn::setNull(offset_t nodeOffset) {
     auto walPageInfo = createWALVersionOfPageForValue(nodeOffset);
     try {
-        propertyStatistics.setHasNull();
+        propertyStatistics.setHasNull(DUMMY_WRITE_TRANSACTION);
         NullMask::setNull((uint64_t*)walPageInfo.frame, walPageInfo.posInPage, true);
     } catch (Exception& e) {
         bufferManager->unpin(*wal->fileHandle, walPageInfo.pageIdxInWAL);
@@ -506,7 +507,7 @@ void NullNodeColumn::writeInternal(
     offset_t nodeOffset, ValueVector* vectorToWriteFrom, uint32_t posInVectorToWriteFrom) {
     writeValue(nodeOffset, vectorToWriteFrom, posInVectorToWriteFrom);
     if (NullMask::isNull(vectorToWriteFrom->getNullMaskData(), posInVectorToWriteFrom)) {
-        propertyStatistics.setHasNull();
+        propertyStatistics.setHasNull(DUMMY_WRITE_TRANSACTION);
     }
 }
 

src/storage/store/property_statistics.cpp

@@ -21,23 +21,17 @@ std::unique_ptr<PropertyStatistics> PropertyStatistics::deserialize(
     return std::make_unique<PropertyStatistics>(hasNull);
 }
 
-// In a read-only context, readStatistics should always be non-null
 // Read/write statistics cannot be cached since functions like checkpointInMemoryIfNecessary may
 // overwrite them and invalidate the reference
-bool RWPropertyStats::mayHaveNull() {
-    auto readStatistics = tablesStatistics->getReadPropertyStatisticsForTable(tableID, propertyID);
-    if (readStatistics) {
-        return readStatistics->mayHaveNull();
-    }
-
-    auto writeStatistics =
-        tablesStatistics->getWritePropertyStatisticsForTable(tableID, propertyID);
-    return writeStatistics.mayHaveNull();
+bool RWPropertyStats::mayHaveNull(const transaction::Transaction& transaction) {
+    auto statistics =
+        tablesStatistics->getPropertyStatisticsForTable(transaction, tableID, propertyID);
+    return statistics.mayHaveNull();
 }
 
-void RWPropertyStats::setHasNull() {
+void RWPropertyStats::setHasNull(const transaction::Transaction& transaction) {
     auto writeStatistics =
-        tablesStatistics->getWritePropertyStatisticsForTable(tableID, propertyID);
+        tablesStatistics->getPropertyStatisticsForTable(transaction, tableID, propertyID);
     writeStatistics.setHasNull();
 }
 

test/common/null_mask_test.cpp

@@ -79,3 +79,16 @@ TEST(NullMaskTests, CopyNullMaskOffsetInvert) {
     // word 4 should be all set, as the inverted source
     ASSERT_EQ(dest[4], ~0);
 }
+
+TEST(NullMaskTests, CopyNullMaskReturnValue) {
+    std::vector<uint64_t> emptySource(10, 0);
+    std::vector<uint64_t> source(10, ~0ull);
+    std::vector<uint64_t> dest(10);
+    ASSERT_EQ(NullMask::copyNullMask(source.data(), 0, dest.data(), 0, 64, false /*invert*/), true);
+    ASSERT_EQ(
+        NullMask::copyNullMask(emptySource.data(), 0, dest.data(), 0, 64, false /*invert*/), false);
+
+    ASSERT_EQ(NullMask::copyNullMask(source.data(), 0, dest.data(), 0, 64, true /*invert*/), false);
+    ASSERT_EQ(
+        NullMask::copyNullMask(emptySource.data(), 0, dest.data(), 0, 64, true /*invert*/), true);
+}