leaves12138 commented on code in PR #446:
URL: https://github.com/apache/paimon-rust/pull/446#discussion_r3518471472
##########
crates/paimon/src/table/full_text_search_builder.rs:
##########
@@ -166,38 +183,255 @@ async fn evaluate_full_text_search(
return Ok(Vec::new());
}
- let futures: Vec<_> = fulltext_entries
- .into_iter()
- .map(|entry| {
- let global_meta =
entry.index_file.global_index_meta.as_ref().unwrap();
- let path = format!("{table_path}/{INDEX_DIR}/{}",
entry.index_file.file_name);
- let file_name = entry.index_file.file_name.clone();
- let query_text = search.query_text.clone();
- let limit = search.limit;
- let row_range_start = global_meta.row_range_start;
- let input = file_io.new_input(&path);
- async move {
- let input = input?;
- let reader = TantivyFullTextReader::from_input_file(&input)
- .await
- .map_err(|e| crate::Error::UnexpectedError {
- message: format!(
- "Failed to open Tantivy full-text index '{}': {}",
- file_name, e
- ),
- source: None,
- })?;
- let result = reader.search(&query_text, limit)?;
- Ok::<_, crate::Error>(result.offset(row_range_start))
- }
- })
- .collect();
-
- let results = futures::future::try_join_all(futures).await?;
let mut merged = SearchResult::empty();
- for r in &results {
- merged = merged.or(r);
+ if !fulltext_entries.is_empty() {
+ let futures: Vec<_> = fulltext_entries
+ .into_iter()
+ .map(|entry| {
+ let global_meta =
entry.index_file.global_index_meta.as_ref().unwrap();
+ let path = format!("{table_path}/{INDEX_DIR}/{}",
entry.index_file.file_name);
+ let file_name = entry.index_file.file_name.clone();
+ let query_text = search.query_text.clone();
+ let limit = search.limit;
+ let row_range_start = global_meta.row_range_start;
+ let input = file_io.new_input(&path);
+ async move {
+ let input = input?;
+ let reader = TantivyFullTextReader::from_input_file(&input)
+ .await
+ .map_err(|e| crate::Error::UnexpectedError {
+ message: format!(
+ "Failed to open Tantivy full-text index '{}':
{}",
+ file_name, e
+ ),
+ source: None,
+ })?;
+ let result = reader.search(&query_text, limit)?;
+ Ok::<_, crate::Error>(result.offset(row_range_start))
+ }
+ })
+ .collect();
+
+ let results = futures::future::try_join_all(futures).await?;
+ for r in &results {
+ merged = merged.or(r);
+ }
+ }
+
+ if search_mode != GlobalIndexSearchMode::Fast {
Review Comment:
This raw fallback is currently unreachable when `fulltext_entries` is empty,
because the function returns `Ok(Vec::new())` earlier. In `full`/`detail` mode,
no Tantivy coverage for this field means all rows are unindexed and should be
searched through the raw full-text path, matching `GlobalIndexCoverage`
semantics and the vector path (`vector_entries.is_empty() && search_mode ==
Fast` is the only early return there). As written, a table configured with
`global-index.search-mode=full` or `detail` but with no full-text index for
`body` returns `[]` instead of matching raw rows. The same issue exists one
layer above when `execute` returns early on a snapshot without an index
manifest. Could we make these early returns apply only to `Fast`, route the
empty/no-manifest case through `unindexed_ranges_for_global_index_entries` +
`read_raw_full_text_search`, and add a regression test similar to the vector
raw-path test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
