fix: minor things

Author: Totodore

quickwit/quickwit-proto/protos/quickwit/search.proto

@@ -125,9 +125,9 @@ message ListFieldsRequest {
   optional int64 start_timestamp = 3;
   optional int64 end_timestamp = 4;
 
-  uint64 max_hits = 5;
+  uint64 max_fields = 5;
   uint64 start_offset = 6;
-  repeated SortField sort_fields = 7;
+  repeated ListFieldSortOrder sort_order = 7;
 
   // Control if the request will fail if split_ids contains a split that does not exist.
   // optional bool fail_on_missing_index = 6;
@@ -170,6 +170,12 @@ message ListFieldsEntryResponse {
   repeated string non_aggregatable_index_ids = 7;
 }
 
+enum ListFieldSortOrder {
+    ALPHABETICAL = 0;
+    OCCURRENCE = 1;
+    TYPE = 2;
+}
+
 enum ListFieldType {
     STR = 0;
     U64 = 1;

quickwit/quickwit-proto/src/codegen/quickwit/quickwit.search.rs

@@ -15,7 +15,7 @@
 use std::collections::{HashMap, HashSet};
 use std::path::Path;
 use std::str::FromStr;
-use std::sync::{Arc, LazyLock};
+use std::sync::Arc;
 
 use anyhow::Context;
 use futures::future;
@@ -27,8 +27,8 @@ use quickwit_common::uri::Uri;
 use quickwit_metastore::SplitMetadata;
 use quickwit_proto::metastore::MetastoreServiceClient;
 use quickwit_proto::search::{
-    LeafListFieldsRequest, ListFields, ListFieldsEntryResponse, ListFieldsRequest,
-    ListFieldsResponse, SplitIdAndFooterOffsets, deserialize_split_fields,
+    LeafListFieldsRequest, ListFieldSortOrder, ListFields, ListFieldsEntryResponse,
+    ListFieldsRequest, ListFieldsResponse, SplitIdAndFooterOffsets, deserialize_split_fields,
 };
 use quickwit_proto::types::{IndexId, IndexUid};
 use quickwit_storage::Storage;
@@ -41,16 +41,6 @@ use crate::{
     search_thread_pool,
 };
 
-/// QW_FIELD_LIST_SIZE_LIMIT defines a hard limit on the number of fields that
-/// can be returned (error otherwise).
-///
-/// Having many fields can happen when a user is creating fields dynamically in
-/// a JSON type with random field names. This leads to huge memory consumption
-/// when building the response. This is a workaround until a way is found to
-/// prune the long tail of rare fields.
-static FIELD_LIST_SIZE_LIMIT: LazyLock<usize> =
-    LazyLock::new(|| quickwit_common::get_from_env("QW_FIELD_LIST_SIZE_LIMIT", 100_000, false));
-
 const DYNAMIC_FIELD_PREFIX: &str = "_dynamic.";
 
 /// Get the list of fields in the given split.
@@ -231,9 +221,8 @@ struct ListFieldMerger<I: Iterator<Item = ListFieldsEntryResponse>> {
 
 impl<I: Iterator<Item = ListFieldsEntryResponse>> ListFieldMerger<I> {
     fn new(iterators: impl Iterator<Item = I>) -> Self {
-        //TODO: sort
         let cmp_fn: fn(&ListFieldsEntryResponse, &ListFieldsEntryResponse) -> bool =
-            |a, b| (&a.field_name, a.field_type) <= (&b.field_name, b.field_type);
+            |a, b| field_order(a, b) == std::cmp::Ordering::Less;
 
         let merged = iterators.kmerge_by(cmp_fn);
         Self {
@@ -396,6 +385,34 @@ pub struct IndexMetasForLeafSearch {
     pub index_uri: Uri,
 }
 
+/// Builds a comparison function for sorting list fields entries.
+///
+/// When multiple sort orders are provided, they are applied in sequence (primary, secondary, etc.).
+/// A final tiebreaker on `(field_name, field_type)` is always appended to ensure deterministic
+/// ordering.
+fn sort_list_fields(fields: &mut [ListFieldsEntryResponse], sort_orders: &[i32]) {
+    fields.sort_unstable_by(|left, right| {
+        let mut ordering = std::cmp::Ordering::Equal;
+        for sort_order in sort_orders {
+            ordering = ordering.then_with(|| {
+                match ListFieldSortOrder::try_from(*sort_order) {
+                    Ok(ListFieldSortOrder::Alphabetical) => left.field_name.cmp(&right.field_name),
+                    Ok(ListFieldSortOrder::Occurrence) => {
+                        // Descending: more occurrences first
+                        right.index_ids.len().cmp(&left.index_ids.len())
+                    }
+                    Ok(ListFieldSortOrder::Type) => left.field_type.cmp(&right.field_type),
+                    Err(_) => std::cmp::Ordering::Equal,
+                }
+            });
+        }
+        // Tiebreaker: deterministic ordering by (field_name, field_type)
+        ordering
+            .then_with(|| left.field_name.cmp(&right.field_name))
+            .then_with(|| left.field_type.cmp(&right.field_type))
+    });
+}
+
 /// Performs a distributed list fields request.
 /// 1. Sends leaf requests over gRPC to multiple leaf nodes.
 /// 2. Merges the search results.
@@ -460,19 +477,15 @@ pub async fn root_list_fields(
             let fields_iter = leaf_list_fields_protos
                 .into_iter()
                 .map(|leaf_list_fields_proto| leaf_list_fields_proto.fields.into_iter());
-            let mut fields_iter = ListFieldMerger::new(fields_iter);
-            let skipped = fields_iter
-                .by_ref()
-                .take(list_fields_req.start_offset as usize)
-                .count();
-
-            let fields = fields_iter
-                .by_ref()
-                .take(list_fields_req.max_fields as usize)
-                .collect::<Vec<_>>();
+            let mut all_fields: Vec<ListFieldsEntryResponse> =
+                ListFieldMerger::new(fields_iter).collect();
+
+            sort_list_fields(&mut all_fields, &list_fields_req.sort_order);
 
-            let remaining = fields_iter.count();
-            let num_fields = (skipped + fields.len() + remaining) as u64;
+            let num_fields = all_fields.len() as u64;
+            let start = list_fields_req.start_offset.min(num_fields) as usize;
+            let end = (start as u64 + list_fields_req.max_fields).min(num_fields) as usize;
+            let fields = all_fields[start..end].to_vec();
 
             (fields, num_fields)
         })

quickwit/quickwit-search/src/list_fields.rs

@@ -17,7 +17,9 @@ use std::sync::Arc;
 
 use percent_encoding::percent_decode_str;
 use quickwit_config::validate_index_id_pattern;
-use quickwit_proto::search::{CountHits, ListFieldsRequest, SortField, SortOrder};
+use quickwit_proto::search::{
+    CountHits, ListFieldSortOrder, ListFieldsRequest, SortField, SortOrder,
+};
 use quickwit_query::query_ast::query_ast_from_user_text;
 use quickwit_search::{SearchError, SearchPlanResponseRest, SearchResponseRest, SearchService};
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
@@ -44,6 +46,7 @@ use crate::{BodyFormat, with_arg};
         SearchResponseRest,
         SearchPlanResponseRest,
         ListFieldsQueryString,
+        ListFieldSortOrderParam,
         SortBy,
         SortField,
         SortOrder,
@@ -145,6 +148,26 @@ fn default_max_hits() -> u64 {
     20
 }
 
+/// Sort order for the list_fields endpoint.
+#[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Serialize, Deserialize, utoipa::ToSchema)]
+#[serde(rename_all = "snake_case")]
+pub enum ListFieldSortOrderParam {
+    #[default]
+    Alphabetical,
+    Occurrence,
+    Type,
+}
+
+impl From<ListFieldSortOrderParam> for ListFieldSortOrder {
+    fn from(param: ListFieldSortOrderParam) -> Self {
+        match param {
+            ListFieldSortOrderParam::Alphabetical => ListFieldSortOrder::Alphabetical,
+            ListFieldSortOrderParam::Occurrence => ListFieldSortOrder::Occurrence,
+            ListFieldSortOrderParam::Type => ListFieldSortOrder::Type,
+        }
+    }
+}
+
 /// This struct represents the QueryString passed to
 /// the rest API.
 #[derive(
@@ -275,13 +298,9 @@ pub struct ListFieldsQueryString {
     /// The output format.
     #[serde(default)]
     pub format: BodyFormat,
-    /// Specifies how documents are sorted.
-    #[serde(alias = "sort_by_field")]
-    #[serde(deserialize_with = "sort_by_mini_dsl")]
+    /// Specifies how fields are sorted. Possible values: "alphabetical", "occurrence", "type".
     #[serde(default)]
-    #[serde(skip_serializing_if = "SortBy::is_empty")]
-    #[param(value_type = String)]
-    pub sort_by: SortBy,
+    pub sort_order: ListFieldSortOrderParam,
 }
 
 pub fn search_request_from_api_request(
@@ -422,7 +441,7 @@ async fn list_fields(
         },
         index_id_patterns,
         max_fields: request.max_fields,
-        sort_fields: request.sort_by.sort_fields,
+        sort_order: vec![ListFieldSortOrder::from(request.sort_order) as i32],
         start_offset: request.start_offset,
     };
     let result = search_service.root_list_fields(req).await;