add list_fields api by PSeitz · Pull Request #4242 · quickwit-oss/quickwit

PSeitz · 2023-12-06T07:35:21Z

Retry
List fields from tantivy
Fallback to schema if field list does not exist
{index_id}/_field_caps URL
/_field_caps URL
fields filter (my-index-000001/_field_caps?fields=rating)
API tests
Caching (very simple list field cache)
Parallel fetch of field list file

➜  quickwit-indices curl -XGET http://localhost:7280/api/v1/_elastic/hdfs/_field_caps -H 'Content-Type: application/json' -d ''
{
  "indices": [
    "hdfs"
  ],
  "fields": {
    "_dynamic.attributes.class": {
      "keyword": {
        "metadata_field": false,
        "searchable": true,
        "aggregatable": true,
        "indices": [
          "hdfs"
        ]
      },
      "text": {
        "metadata_field": false,
        "searchable": true,
        "aggregatable": true,
        "indices": [
          "hdfs"
        ]
      }
    },
    "_field_presence": {
      "long": {
        "metadata_field": false,
        "searchable": true,
        "aggregatable": false,
        "indices": [
          "hdfs"
        ]
      }
    },
    "body": {
      "text": {
        "metadata_field": false,
        "searchable": true,
        "aggregatable": false,
        "indices": [
          "hdfs"
        ]
      }
    },
    "timestamp": {
      "date_nanos": {
        "metadata_field": false,
        "searchable": true,
        "aggregatable": true,
        "indices": [
          "hdfs"
        ]
      }
    },
    "tenant_id": {
      "long": {
        "metadata_field": false,
        "searchable": true,
        "aggregatable": false,
        "indices": [
          "hdfs"
        ]
      }
    },
    "resource.$facet:service": {
      "text": {
        "metadata_field": false,
        "searchable": true,
        "aggregatable": false,
        "indices": [
          "hdfs"
        ]
      }
    },
    "severity_text": {
      "keyword": {
        "metadata_field": false,
        "searchable": true,
        "aggregatable": true,
        "indices": [
          "hdfs"
        ]
      },
      "text": {
        "metadata_field": false,
        "searchable": true,
        "aggregatable": true,
        "indices": [
          "hdfs"
        ]
      }
    }
  }
}⏎

fulmicoton · 2023-12-06T08:14:54Z

+  // The field name
+  string field_name = 1;
+  // The tantivy field type
+  int32 field_type = 2;


Can you comment on how it is mapped? Why signed int?

Suggested change

int32 field_type = 2;

int32 field_type = 2;

I added a comment (Type::to_code()) and switched to u32

fulmicoton · 2023-12-06T08:15:37Z

+  // False means the field is not searchable in any indices.
+  bool searchable = 4;
+  // True means the field is aggregatable (fast) in at least some indices.
+  // False means the field is not aggregatable in any indices.


do you really mean indices in all of this proto, or did you mean split ids?

yes indices. There is no use for split ids in the API currently

fulmicoton · 2023-12-06T08:21:52Z

+            index_ids: vec![index_id.to_string()],
+            searchable: metadata.indexed,
+            aggregatable: metadata.fast,
+            non_searchable_index_ids: Vec::new(),


I don't understand this contract.
So you return io::Result and lose the info of the missing split ids?
What are non_searchable_index_ids ?

When do we return an ioError as an item.

Actually I'm not sure what to do with the error, currently we abort and return. This is happening in the merge function. I guess we want to return the failed split_ids for a retry. There's no retry currently.

let entry = entry.map_err(|err| crate::error::SearchError::Internal(format!("{:?}", err)))?; // TODO: No early return on error

non_searchable_index_ids are exemption list of indexes if searchable: true. I took this from the elastic search API.

fulmicoton · 2023-12-11T08:04:03Z

 quickwit-proto = { workspace = true }
 quickwit-query = { workspace = true }
 quickwit-storage = { workspace = true }
+quickwit-indexing = { workspace = true }


do we really need this?

quickwit-indexing contains the code to write and read the split fields. I could move it, but not sure where to

fulmicoton · 2023-12-11T08:08:45Z

+    let aggregatable = current_group.iter().any(|entry| entry.aggregatable);
+    let field_name = metadata.field_name.to_string();
+    let field_type = metadata.field_type;
+    let mut non_searchable_index_ids = if searchable {


Can you add the type here?

Suggested change

let mut non_searchable_index_ids = if searchable {

let mut non_searchable_index_ids = if searchable {

Also if perf do not matter, BTreeSet<&str> is a simple way to inform users and maintain the contract that this is sorted and unique.

This is building the protobuf API response, which requires Vec

fulmicoton · 2023-12-12T06:12:10Z

+# Test date _field_caps API
+method: [GET]
+engines:
+  - quickwit


how far are we from elastic's api?

The most important features are supported, some minor parameters are not implemented yet

if we drop

engines: - quickwit

which part breaks?

it works without, seems to pick quickwit as default

I mean running it against elasticsearch without it.

The main differences are:

Mixed types are not supported in elastic, so this test doesn't work in elastic. We get multiple entries for mixed types. Support elasticsearch field capabilities endpoint #3527 (comment)

We have different coercion rules (e.g. i64, u64, f64 preference order)

Mapping of text types. Currently we map text fields with fast to keyword and indexed to text. keyword is not on the same fieldname in elastic but a new one.

We map our floats to double, but elastic seems to use float when the type is detected

fulmicoton · 2023-12-12T06:24:22Z

+  // The field name
+  string field_name = 1;
+  // The tantivy field type mapped via `to_code`.
+  uint32 field_type = 2;


make it a proto enum to remove the dependency in quickwit-serve.

I would rather reuse the existing code, tantivy is in the dependency tree anyway

fulmicoton

The code goes

serialize list of fields as a file artifact with code living in quickwit-indexing, and using tantivy's types.
convert artifact to protobuf

Would using protobuf to begin lead to better code?
We could remove code, and drop the quickwit-search -> quickwit-indexing dependency

PSeitz · 2023-12-12T07:12:31Z

The code goes
* serialize list of fields as a file artifact with code living in quickwit-indexing, and using tantivy's types.

* convert artifact to protobuf
Would using protobuf to begin lead to better code? We could remove code, and drop the quickwit-search -> quickwit-indexing dependency

I don't think this would be better code, since this would be creating duplicated code. The existing type does exactly what we want and tantivy is already in the dependency tree.
The quickwit-search -> quickwit-indexing dependency has nothing to do with the tantivy types, this is because field de/serialziation is in models/list_fields.rs

fmassot · 2023-12-14T23:24:16Z

+        };
+        let list_fields_iter = list_fields_iter.filter(|field| {
+            if let Ok(field) = field {
+                // We don't want to leak the _dynamic hack to the user API.


I think we want to remove the _dynamic from the field names as we don't want to leak that to the users.

Yes, I think so too. I wasn't completely sure if there's a case where we need it.

handle wildcards handle _dynamic special case

PSeitz force-pushed the list_fields_api branch from 13d3418 to cbed13d Compare December 6, 2023 07:36