From 363e1c2ef98cd0be8caee8817415c1d54f0d8196 Mon Sep 17 00:00:00 2001
From: Pascal Seitz <pascal.seitz@gmail.com>
Date: Fri, 31 May 2024 13:35:38 +0800
Subject: [PATCH] add comments

---
 quickwit/quickwit-search/src/collector.rs | 2 +-
 quickwit/quickwit-search/src/leaf.rs      | 4 ++--
 quickwit/quickwit-search/src/root.rs      | 7 +++++--
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/quickwit/quickwit-search/src/collector.rs b/quickwit/quickwit-search/src/collector.rs
index b7a4edfe2d4..97a4a3c75d4 100644
--- a/quickwit/quickwit-search/src/collector.rs
+++ b/quickwit/quickwit-search/src/collector.rs
@@ -723,7 +723,7 @@ pub(crate) struct QuickwitCollector {
 
 impl QuickwitCollector {
     pub fn is_count_only(&self) -> bool {
-        self.max_hits == 0 && self.aggregation.is_none() && self.search_after.is_none()
+        self.max_hits == 0 && self.aggregation.is_none()
     }
     /// Updates search parameters affecting the returned documents.
     /// Does not update aggregations.
diff --git a/quickwit/quickwit-search/src/leaf.rs b/quickwit/quickwit-search/src/leaf.rs
index bc6c3e51a96..f5ce30b7363 100644
--- a/quickwit/quickwit-search/src/leaf.rs
+++ b/quickwit/quickwit-search/src/leaf.rs
@@ -47,7 +47,7 @@ use tantivy::{DateTime, Index, ReloadPolicy, Searcher, Term};
 use tracing::*;
 
 use crate::collector::{make_collector_for_split, make_merge_collector, IncrementalCollector};
-use crate::root::is_metadata_count_request_with_ast;
+use crate::root::{is_metadata_count_request, is_metadata_count_request_with_ast};
 use crate::service::{deserialize_doc_mapper, SearcherContext};
 use crate::{QuickwitAggregations, SearchError};
 
@@ -1038,7 +1038,7 @@ async fn resolve_storage_and_leaf_search(
 }
 
 /// Optimizes the search_request based on CanSplitDoBetter
-/// Returns a tuple of (the search_request was optimized, split can return better results)
+/// Returns true if the split can return better results
 fn check_optimize_search_request(
     search_request: &mut SearchRequest,
     split: &SplitIdAndFooterOffsets,
diff --git a/quickwit/quickwit-search/src/root.rs b/quickwit/quickwit-search/src/root.rs
index 164493bc456..dc28ed8d5e8 100644
--- a/quickwit/quickwit-search/src/root.rs
+++ b/quickwit/quickwit-search/src/root.rs
@@ -609,8 +609,11 @@ pub fn is_metadata_count_request_with_ast(query_ast: &QueryAst, request: &Search
         return false;
     }
 
-    // TODO: if the start and end timestamp encompass the whole split, it is still a count query
-    // So some could be checked on metadata
+    // If the start and end timestamp encompass the whole split, it is still a count query.
+    // We remove this currently on the leaf level, but not yet on the root level.
+    // There's a small advantage when we would do this on the root level, since we have the
+    // counts available on the split. On the leaf it is currently required to open the split
+    // to get the count.
     if request.start_timestamp.is_some() || request.end_timestamp.is_some() {
         return false;
     }