Fix abort on inverted time-travel range (#890)

Time-travel queries are supposed to return an empty result set when the time range is empty. This extends to inverted ranges as well, i.e. an empty result set is always returned when the start time is after the end time. However, the underlying TileDB query actually aborts when an inverted range is given. This was usually missed by CI because the test was so small that the start and end wall clock times used were the same, resulting in the expected behaviour. This commit fixes the issue by short circuiting and skipping the TileDB query altogether when the range is inverted.

Author: alancleary

libtiledbvcf/src/dataset/tiledbvcfdataset.cc

@@ -29,6 +29,7 @@
 #include <cstdlib>
 #include <future>
 #include <map>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -571,12 +572,12 @@ void TileDBVCFDataset::open(
   // 'vcf.end_timestamp' in tiledb_config. If either timestamp is provided,
   // reopen the arrays.
   bool reopen = false;
+  std::optional<uint64_t> start_timestamp;
+  std::optional<uint64_t> end_timestamp;
 
   try {
-    auto start_timestamp = std::stoull(cfg_.get("vcf.start_timestamp"));
-    data_array_->set_open_timestamp_start(start_timestamp);
-    vcf_header_array_->set_open_timestamp_start(start_timestamp);
-    LOG_INFO("Using vcf.start_timestamp from config: {}", start_timestamp);
+    start_timestamp = std::stoull(cfg_.get("vcf.start_timestamp"));
+    LOG_INFO("Using vcf.start_timestamp from config: {}", *start_timestamp);
     reopen = true;
   } catch (const tiledb::TileDBError& ex) {
     LOG_TRACE("'vcf.start_timestamp' not specified in config, using default");
@@ -587,10 +588,8 @@ void TileDBVCFDataset::open(
   }
 
   try {
-    auto end_timestamp = std::stoull(cfg_.get("vcf.end_timestamp"));
-    data_array_->set_open_timestamp_end(end_timestamp);
-    vcf_header_array_->set_open_timestamp_end(end_timestamp);
-    LOG_INFO("Using vcf.end_timestamp from config: {}", end_timestamp);
+    end_timestamp = std::stoull(cfg_.get("vcf.end_timestamp"));
+    LOG_INFO("Using vcf.end_timestamp from config: {}", *end_timestamp);
     reopen = true;
   } catch (const tiledb::TileDBError& ex) {
     LOG_TRACE("'vcf.end_timestamp' not specified in config, using default");
@@ -600,6 +599,29 @@ void TileDBVCFDataset::open(
         cfg_.get("vcf.end_timestamp"));
   }
 
+  // If the user supplied an inverted range (start > end), short-circuit:
+  // mark the dataset as having an empty time range and skip the reopen.
+  // Calling reopen() with start > end would surface as an uncaught
+  // "Range is empty" exception in some TileDB versions and abort the process.
+  if (start_timestamp && end_timestamp && *start_timestamp > *end_timestamp) {
+    LOG_INFO(
+        "vcf.start_timestamp ({}) > vcf.end_timestamp ({}); treating as "
+        "empty time range",
+        *start_timestamp,
+        *end_timestamp);
+    empty_time_range_ = true;
+    reopen = false;
+  } else {
+    if (start_timestamp) {
+      data_array_->set_open_timestamp_start(*start_timestamp);
+      vcf_header_array_->set_open_timestamp_start(*start_timestamp);
+    }
+    if (end_timestamp) {
+      data_array_->set_open_timestamp_end(*end_timestamp);
+      vcf_header_array_->set_open_timestamp_end(*end_timestamp);
+    }
+  }
+
   if (reopen) {
     data_array_->reopen();
     vcf_header_array_->reopen();

libtiledbvcf/src/dataset/tiledbvcfdataset.h

@@ -812,6 +812,16 @@ class TileDBVCFDataset {
   /** Returns true if the dataset is tiledb cloud URI. */
   bool tiledb_cloud_dataset() const;
 
+  /**
+   * Returns true if the user requested an inverted time-travel range
+   * (vcf.start_timestamp > vcf.end_timestamp). In that case the arrays were
+   * never reopened with the inverted range, and callers should treat reads
+   * as producing zero results.
+   */
+  bool empty_time_range() const {
+    return empty_time_range_;
+  }
+
   /**
    * Gets the datatype of a particular exportable attribute that is not fmt or
    * info
@@ -881,6 +891,11 @@ class TileDBVCFDataset {
   /** Set to true when the dataset is opened. */
   bool open_;
 
+  /** Set to true when open() was called with an inverted time-travel range
+   * (start_timestamp > end_timestamp). When true, reads should short-circuit
+   * to zero results without consulting the underlying arrays. */
+  bool empty_time_range_ = false;
+
   /** The dataset's general metadata (does not contain sample header data). */
   Metadata metadata_;
 

libtiledbvcf/src/read/reader.cc

@@ -365,6 +365,12 @@ void Reader::read() {
     throw std::runtime_error(
         "Error exporting records; reader has not been initialized.");
 
+  if (dataset_->empty_time_range()) {
+    read_state_.status = ReadStatus::COMPLETED;
+    buffers_a.reset(nullptr);
+    return;
+  }
+
   bool pending_work = true;
   switch (read_state_.status) {
     case ReadStatus::COMPLETED:
@@ -2587,6 +2593,10 @@ void Reader::info_attribute_count(int32_t* count) {
 void Reader::sample_count(int32_t* count) {
   if (count == nullptr)
     throw std::runtime_error("Error getting sample count");
+  if (dataset_->empty_time_range()) {
+    *count = 0;
+    return;
+  }
   *count = dataset_->sample_names().size();
 }