(I think) this fixes the Halloween problem

Author: poojanilangekar

src/codegen/updater.cpp

@@ -53,6 +53,11 @@ char *Updater::GetDataPtr(uint32_t tile_group_id, uint32_t tuple_offset) {
 
 char *Updater::Prepare(uint32_t tile_group_id, uint32_t tuple_offset) {
   PELOTON_ASSERT(table_ != nullptr && executor_context_ != nullptr);
+
+  if (IsInStatementWriteSet(ItemPointer(tile_group_id, tuple_offset))) {
+    return nullptr;
+  }
+  
   auto *txn = executor_context_->GetTransaction();
   auto tile_group = table_->GetTileGroupById(tile_group_id).get();
   auto *tile_group_header = tile_group->GetHeader();
@@ -79,7 +84,7 @@ char *Updater::PreparePK(uint32_t tile_group_id, uint32_t tuple_offset) {
 
   PELOTON_ASSERT(table_ != nullptr && executor_context_ != nullptr);
 
-  if (statement_write_set_->find(ItemPointer(tile_group_id, tuple_offset)) != statement_write_set_->end()) {
+  if (IsInStatementWriteSet(ItemPointer(tile_group_id, tuple_offset))) {
     return nullptr;
   }
 
@@ -130,7 +135,6 @@ void Updater::Update() {
   auto tile_group = table_->GetTileGroupById(old_location_.block).get();
   auto *tile_group_header = tile_group->GetHeader();
   auto &txn_manager = concurrency::TransactionManagerFactory::GetInstance();
-
   // Either update in-place
   if (is_owner_ == true) {
     txn_manager.PerformUpdate(txn, old_location_);
@@ -151,6 +155,7 @@ void Updater::Update() {
     return;
   }
   txn_manager.PerformUpdate(txn, old_location_, new_location_);
+  AddToStatementWriteSet(new_location_);
   executor_context_->num_processed++;
 }
 
@@ -174,7 +179,7 @@ void Updater::UpdatePK() {
     return;
   }
   txn_manager.PerformInsert(txn, new_location_, index_entry_ptr);
-  statement_write_set_->insert(new_location_);
+  AddToStatementWriteSet(new_location_);
   executor_context_->num_processed++;
 }
 

src/executor/update_executor.cpp

@@ -62,10 +62,10 @@ bool UpdateExecutor::PerformUpdatePrimaryKey(
     bool is_owner, storage::TileGroup *tile_group,
     storage::TileGroupHeader *tile_group_header, oid_t physical_tuple_id,
     ItemPointer &old_location) {
-  
-  if (statement_write_set_.find(old_location) != statement_write_set_.end()) {
+
+  /* if (statement_write_set_.find(old_location) != statement_write_set_.end()) {
     return true;
-  }
+  } */
 
   auto &transaction_manager =
       concurrency::TransactionManagerFactory::GetInstance();
@@ -195,6 +195,9 @@ bool UpdateExecutor::DExecute() {
     oid_t physical_tuple_id = pos_lists[0][visible_tuple_id];
 
     ItemPointer old_location(tile_group->GetTileGroupId(), physical_tuple_id);
+    if (IsInStatementWriteSet(old_location)) {
+      continue;
+    }
 
     LOG_TRACE("Visible Tuple id : %u, Physical Tuple id : %u ",
               visible_tuple_id, physical_tuple_id);
@@ -269,6 +272,7 @@ bool UpdateExecutor::DExecute() {
                                 executor_context_);
 
         transaction_manager.PerformUpdate(current_txn, old_location);
+        statement_write_set_.insert(old_location);
       }
     }
     // if we have already obtained the ownership
@@ -366,6 +370,7 @@ bool UpdateExecutor::DExecute() {
                     new_location.offset);
           transaction_manager.PerformUpdate(current_txn, old_location,
                                             new_location);
+          statement_write_set_.insert(new_location);
 
           // TODO: Why don't we also do this in the if branch above?
           executor_context_->num_processed += 1;  // updated one

src/include/codegen/updater.h

@@ -67,6 +67,17 @@ class Updater {
 
   char *GetDataPtr(uint32_t tile_group_id, uint32_t tuple_offset);
 
+  // Check if the tuple is in the statement write set
+  inline bool IsInStatementWriteSet(ItemPointer location) {
+    return statement_write_set_->find(location) !=
+           statement_write_set_->end();
+  }
+
+  // Add the updated location to the statement write set
+  inline void AddToStatementWriteSet(ItemPointer& location) {
+    statement_write_set_->insert(location);
+  }
+
  private:
   // Table and executor context from the update translator
   storage::DataTable *table_;

src/include/executor/update_executor.h

@@ -49,6 +49,10 @@ class UpdateExecutor : public AbstractExecutor {
 
   bool DExecute();
 
+  inline bool IsInStatementWriteSet(ItemPointer &location) {
+    return (statement_write_set_.find(location) != statement_write_set_.end());
+  }
+
  private:
   storage::DataTable *target_table_ = nullptr;
   const planner::ProjectInfo *project_info_ = nullptr;