diff --git a/src/service/search/grpc/storage.rs b/src/service/search/grpc/storage.rs index c897a7314b..b05f8f0c5f 100644 --- a/src/service/search/grpc/storage.rs +++ b/src/service/search/grpc/storage.rs @@ -413,12 +413,7 @@ pub async fn cache_files( } } -/// Filter file list using inverted index -/// This function will load the index file corresponding to each file in the file list. -/// FSTs in those files are used to match the incoming query in `SearchRequest`. -/// If the query does not match any FST in the index file, the file will be filtered out. -/// If the query does match then the segment IDs for the file will be updated. -/// If the query not find corresponding index file, the file will *not* be filtered out. +/// Filter file list using tantivy index #[tracing::instrument(name = "service:search:grpc:storage:tantivy_search", skip_all)] pub async fn tantivy_search( query: Arc, @@ -600,11 +595,8 @@ pub async fn tantivy_search( // Each result corresponds to a file in the file list match result { Ok((file_name, result, has_skipped_conditions)) => { - // If any AND condition was skipped because its field does - // not exist in this tantivy index (e.g., a newly added - // secondary-index field with no historical data), keep the - // DataFusion filter so it can evaluate the skipped - // predicates on the parquet data. + // when has_skipped_conditions is true, we should add filter back to datafusion, + // because the index result is not accurate if has_skipped_conditions { is_add_filter_back = true; } @@ -726,10 +718,7 @@ pub async fn get_tantivy_directory( } /// Returns (file_key, result, has_skipped_conditions). -/// `has_skipped_conditions` is true when one or more AND conditions were skipped -/// because the corresponding field does not exist in this tantivy index (e.g., a -/// newly added secondary-index field). The caller must preserve the DataFusion -/// filter to handle those skipped predicates. +/// when has_skipped_conditions is true, we should all filter back to datafusion. async fn search_tantivy_index( trace_id: &str, time_range: (i64, i64), @@ -807,7 +796,7 @@ async fn search_tantivy_index( let condition: IndexCondition = index_condition.ok_or(anyhow::anyhow!("IndexCondition not found"))?; let (query, has_skipped_conditions) = - condition.to_tantivy_query(tantivy_schema.clone(), fts_field)?; + condition.to_tantivy_query(trace_id, tantivy_schema.clone(), fts_field)?; let need_all_term_fields = condition .need_all_term_fields() .into_iter() diff --git a/src/service/search/index.rs b/src/service/search/index.rs index 753b57450d..ef08b736d7 100644 --- a/src/service/search/index.rs +++ b/src/service/search/index.rs @@ -134,6 +134,7 @@ impl IndexCondition { // so that the skipped predicates are still evaluated. pub fn to_tantivy_query( &self, + trace_id: &str, schema: Schema, default_field: Option, ) -> anyhow::Result<(Box, bool)> { @@ -145,20 +146,16 @@ impl IndexCondition { queries.push((Occur::Must, query)); } Err(e) => { - // Field not present in this tantivy index file (e.g., a - // secondary index field that was added after this file was - // created). Skip the condition and let DataFusion handle it. - log::debug!("to_tantivy_query: skipping condition due to missing field: {e}"); + log::info!( + "[trace_id {trace_id}] to_tantivy_query: skipping condition due to error: {e}" + ); has_skipped = true; } } } if queries.is_empty() { - // All conditions were skipped because none of the queried fields - // exist in this tantivy index. Return an error so the caller falls - // back to a direct parquet scan instead of doing a full-index scan. Err(anyhow::anyhow!( - "All AND conditions reference fields missing from the tantivy index" + "All AND conditions are failed to generate tantivy query" )) } else { Ok((Box::new(BooleanQuery::from(queries)), has_skipped)) @@ -2155,7 +2152,7 @@ mod tests { cond.add_condition(Condition::Equal("B".into(), "b".into())); let (query, has_skipped) = cond - .to_tantivy_query(schema, None) + .to_tantivy_query("test", schema, None) .expect("query build should succeed"); assert!(!has_skipped, "no field is missing, should not skip"); @@ -2176,7 +2173,7 @@ mod tests { cond.add_condition(Condition::Equal("B".into(), "b".into())); let (query, has_skipped) = cond - .to_tantivy_query(schema, None) + .to_tantivy_query("test", schema, None) .expect("query build should succeed even when a field is missing"); assert!(has_skipped, "missing field B should be reported as skipped"); @@ -2195,7 +2192,7 @@ mod tests { cond.add_condition(Condition::Equal("A".into(), "a".into())); cond.add_condition(Condition::Equal("B".into(), "b".into())); - let result = cond.to_tantivy_query(schema, None); + let result = cond.to_tantivy_query("test", schema, None); assert!( result.is_err(), "should return error when all fields are missing" @@ -2209,7 +2206,7 @@ mod tests { let mut cond = IndexCondition::new(); cond.add_condition(Condition::Equal("B".into(), "b".into())); - let result = cond.to_tantivy_query(schema, None); + let result = cond.to_tantivy_query("test", schema, None); assert!( result.is_err(), "should return error when the only field is missing"