Addressing feedback from @ypatia.

tiledb/sm/query/readers/reader_base.cc

@@ -80,7 +80,7 @@ ReaderBase::ReaderBase(
     , use_timestamps_(false) {
   if (array != nullptr)
     fragment_metadata_ = array->fragment_metadata();
-  timestamps_for_deletes_.resize(fragment_metadata_.size());
+  timestamps_needed_for_deletes_.resize(fragment_metadata_.size());
 }
 
 /* ********************************* */
@@ -169,7 +169,7 @@ bool ReaderBase::need_timestamped_conditions() {
         if (delete_timestamp >= frag_timestamps.first &&
             delete_timestamp <= frag_timestamps.second) {
           make_timestamped_conditions = true;
-          timestamps_for_deletes_[i] = true;
+          timestamps_needed_for_deletes_[i] = true;
         }
       }
     }
@@ -182,7 +182,12 @@ Status ReaderBase::generate_timestamped_conditions() {
   // Generate timestamped conditions.
   timestamped_delete_conditions_.reserve(delete_conditions_.size());
   for (auto& delete_condition : delete_conditions_) {
-    // Make the timestamp condition.
+    // We want the condition to be:
+    // DELETE WHERE (cond) AND cell timestamp <= delete timestamp.
+    // For apply, this condition needs to be be negated and become:
+    // (!cond) OR cell timestamp > delete timestamp.
+
+    // Make the timestamp condition, cell timestamp > delete timestamp.
     QueryCondition timestamp_condition;
     auto delete_timestamp = delete_condition.condition_timestamp();
     std::string attr = constants::timestamps;
@@ -192,7 +197,8 @@ Status ReaderBase::generate_timestamped_conditions() {
         constants::timestamp_size,
         QueryConditionOp::GT));
 
-    // Combine the timestamp condition and delete condition.
+    // Combine the timestamp condition and delete condition. The condition is
+    // already negated.
     QueryCondition timestamped_condition;
     RETURN_NOT_OK(timestamp_condition.combine(
         delete_condition,
@@ -326,7 +332,7 @@ bool ReaderBase::include_timestamps(const unsigned f) const {
   auto dups = array_schema_.allows_dups();
 
   return frag_has_ts && (user_requested_timestamps_ || partial_overlap ||
-                         !dups || timestamps_for_deletes_[f]);
+                         !dups || timestamps_needed_for_deletes_[f]);
 }
 
 Status ReaderBase::load_tile_offsets(

tiledb/sm/query/readers/reader_base.h

@@ -191,9 +191,9 @@ class ReaderBase : public StrategyBase {
 
   /**
    * Boolean, per fragment, to specify that we need to load timestamps for
-   * deletes.
+   * deletes. This matches the fragments in 'fragment_metadata_'
    */
-  std::vector<bool> timestamps_for_deletes_;
+  std::vector<bool> timestamps_needed_for_deletes_;
 
   /* ********************************* */
   /*         PROTECTED METHODS         */

tiledb/sm/query/readers/sparse_global_order_reader.cc

@@ -1359,7 +1359,7 @@ SparseGlobalOrderReader<BitmapType>::respect_copy_memory_budget(
         // For dimensions or query condition fields, tiles are already all
         // loaded in memory.
         if (array_schema_.is_dim(name) ||
-            qc_loaded_names_set_.count(name) != 0 || is_timestamps)
+            qc_loaded_attr_names_set_.count(name) != 0 || is_timestamps)
           return Status::Ok();
 
         // Get the size for all tiles.
@@ -1638,7 +1638,7 @@ Status SparseGlobalOrderReader<BitmapType>::process_slabs(
         *buffers_[name].validity_vector_.buffer_size() = total_cells;
 
       // Clear tiles from memory.
-      if (!is_dim && qc_loaded_names_set_.count(name) == 0 &&
+      if (!is_dim && qc_loaded_attr_names_set_.count(name) == 0 &&
           name != constants::timestamps &&
           name != constants::delete_timestamps) {
         clear_tiles(name, result_tiles);

tiledb/sm/query/readers/sparse_index_reader_base.cc

@@ -189,8 +189,8 @@ SparseIndexReaderBase::get_coord_tiles_size(
 
   // Compute query condition tile sizes.
   uint64_t tiles_size_qc = 0;
-  if (!qc_loaded_names_.empty()) {
-    for (auto& name : qc_loaded_names_) {
+  if (!qc_loaded_attr_names_.empty()) {
+    for (auto& name : qc_loaded_attr_names_) {
       // Calculate memory consumption for this tile.
       auto&& [st, tile_size] = get_attribute_tile_size(name, f, t);
       RETURN_NOT_OK_TUPLE(st, nullopt);
@@ -223,21 +223,21 @@ Status SparseIndexReaderBase::load_initial_data(bool include_coords) {
   if (!condition_.empty()) {
     for (auto& name : condition_.field_names()) {
       if (!array_schema_.is_dim(name) || !include_coords) {
-        qc_loaded_names_set_.insert(name);
+        qc_loaded_attr_names_set_.insert(name);
       }
     }
   }
   for (auto delete_condition : delete_conditions_) {
     for (auto& name : delete_condition.field_names()) {
       if (!array_schema_.is_dim(name) || !include_coords) {
-        qc_loaded_names_set_.insert(name);
+        qc_loaded_attr_names_set_.insert(name);
       }
     }
   }
 
-  qc_loaded_names_.reserve(qc_loaded_names_set_.size());
-  for (auto& name : qc_loaded_names_set_) {
-    qc_loaded_names_.emplace_back(name);
+  qc_loaded_attr_names_.reserve(qc_loaded_attr_names_set_.size());
+  for (auto& name : qc_loaded_attr_names_set_) {
+    qc_loaded_attr_names_.emplace_back(name);
   }
 
   // For easy reference.
@@ -373,12 +373,12 @@ Status SparseIndexReaderBase::read_and_unfilter_coords(
     RETURN_CANCEL_OR_ERROR(unfilter_tiles(constants::timestamps, result_tiles));
   }
 
-  if (!qc_loaded_names_.empty()) {
+  if (!qc_loaded_attr_names_.empty()) {
     // Read and unfilter tiles for query condition.
     RETURN_CANCEL_OR_ERROR(
-        read_attribute_tiles(qc_loaded_names_, result_tiles));
+        read_attribute_tiles(qc_loaded_attr_names_, result_tiles));
 
-    for (const auto& name : qc_loaded_names_) {
+    for (const auto& name : qc_loaded_attr_names_) {
       RETURN_CANCEL_OR_ERROR(unfilter_tiles(name, result_tiles));
     }
   }

tiledb/sm/query/readers/sparse_index_reader_base.h

@@ -295,10 +295,10 @@ class SparseIndexReaderBase : public ReaderBase {
   bool elements_mode_;
 
   /** Names of dim/attr loaded for query condition. */
-  std::vector<std::string> qc_loaded_names_;
+  std::vector<std::string> qc_loaded_attr_names_;
 
   /** Names of dim/attr loaded for query condition. */
-  std::unordered_set<std::string> qc_loaded_names_set_;
+  std::unordered_set<std::string> qc_loaded_attr_names_set_;
 
   /* Are the users buffers full. */
   bool buffers_full_;