[FEATURE] [RFC] Query Shape Field Data Type

[dzane17]

Background

Query shape currently has the ability to append field names. In addition, we'd like the option to add field data type (often called fieldType in code).

In this example, field1, field2, field3 is are field names and text, date, keyword are data types.

bool
  filter:
    terms [field1:text]
  must:
    range [field2:date]
    term [field3:keyword]
  must_not:
    exists [field4:keyword]
  should:
    match [field5:text]
sort:
  asc [field6:number]
  desc [field2:date]
aggregation:
  terms [field3:keyword]
    aggregation:
      avg [field6:number]
      cardinality [field3:keyword]
      date_histogram [field2:date]
      max [field6:number]
      min [field6:number]
      percentile_ranks [field6:number]
      sum [field6:number]

Problem

It is impossible to determine data type from just the search source.

In this match query, field name is "title", however the exact data type is unknown. It could be text or keyword.

GET /my_index/_search
{
  "query": {
    "match_phrase": {
      "title": "Second Document"
    }
  }
}

What solution would you like?

Three possible solutions:

1. Record dataType in each *QueryBuilder, *AggregationBuilder, *SortBuilder class during core search execution. Then when records are processed in query-insights plugin we can simply call aggBuilder.getDataType(). I have seen builder classes get the data type mapping from shardContext like:

final MappedFieldType fieldType = shardContext.fieldMapper(fieldName);

Cons: Not all builders look up data type, Need to edit *Builder classes in core

2. Fetch mappings from query-insights plugin. Then we can get data type from known field name.
   Note: Need to find a way to fetch _mapping data from query-insights

3. Ignore field data type in query shape

In many cases, data type is known given the query/agg/sort type. For example, data type for date histogram aggregations is always date, boolean query is always boolean data type. In these cases, adding data type adds no value:

    aggregation:
      date_histogram [date]

vs. 


    aggregation:
      date_histogram

On the other hand, Range queries support a variety of data types so we would lose information with this option.
a. Date

GET /my_index/_search
{
  "query": {
    "range": {
      "publish_date": {
        "gte": "2024-01-01",
        "lte": "2024-12-31"
      }
    }
  }
}

b. Keyword

GET /my_index/_search
{
  "query": {
    "range": {
      "numeric_as_string": {
        "gte": "10",
        "lte": "100"
      }
    }
  }
}

c. Numeric (which consists of int, long, float, double, short, byte)

GET /my_index/_search
{
  "query": {
    "range": {
      "price": {
        "gte": 10,
        "lte": 100
      }
    }
  }
}

Other Consideration

1. When data type is String or Numeric, do we need to distinguish between specific types (eg. int, long, float, double, short, byte)?

[ansjcy]

Thanks for the RFC!

Not all builders look up data type, Need to edit *Builder classes in core

Do we know what is the coverage? i.e. how many builders and what are the builders have data type available (can we attach a more detailed list here [1] )?

Fetch mappings from query-insights plugin.

This option is not as intrusive as #1, but there are chances the mapping could be outdated. If we want to keep the mapping up to date, we need to constantly fetch and refersh them.

Instead of adding field data type to all query types, we should probably identify the important ones and only add type info for them. For example, as you mentioned we can always add types for range queries if type information can be easily fetched based on researches done in [1].

[dzane17]

@ansjcy Looks like most builders don't already look up data type

Count of all Query/Agg/Sort Builders

% find ./server/src/main/java/org/opensearch -type f -name '*QueryBuilder.java' | wc -l
      50
% find ./server/src/main/java/org/opensearch -type f -name '*AggregationBuilder.java' | wc -l
      62
% find ./server/src/main/java/org/opensearch -type f -name '*SortBuilder.java' | wc -l
       6

Count of Builders which look up data type

% find ./server/src/main/java/org/opensearch -type f -name '*QueryBuilder.java' -exec grep -l 'MappedFieldType' {} + | wc -l
      23
% find ./server/src/main/java/org/opensearch -type f -name '*AggregaionBuilder.java' -exec grep -l 'MappedFieldType' {} + | wc -l
       0
% find ./server/src/main/java/org/opensearch -type f -name '*SortBuilder.java' -exec grep -l 'MappedFieldType' {} + | wc -l
       2

[jainankitk]

@dzane17 - Thank you for putting up this proposal and different possible options. While I am leaning towards option 1, I am wondering about below regarding the same:

• How much work are talking per *Builder class? I am not concerned as much about the number of classes, as I am about how intrusive the change is.
• Are there common/base *Builder class used across multiple subclasses not requiring change in all the subclasses?
• How will be sustain this in future as and when new *Builder class are introduced?

I am slightly less in favor of Option 2, as I see no good reason for Query Insights to have the logic for parsing index mapping.

[dzane17]

@jainankitk

• How much work are talking per *Builder class? I am not concerned as much about the number of classes, as I am about how intrusive the change is.

Query Builders look up field type during the doToQuery() method. We can save this value as a class variable during search execution. Then we will have it when search records are processed in the query insights plugin. This way the change is not very intrusive. Here are some examples of data type lookup in different Builder classes:

• Range Query Builder
• Terms Query Builder
• Wildcard Query Builder
• Field Sort Builder

As mentioned previously, not all *Builder classes currently look up fieldType. In those instances we will not have data type or will need to add a lookup.

• Are there common/base *Builder class used across multiple subclasses not requiring change in all the subclasses?

Yes, we can add the class variable and getter method in some parent class, but we will still need to edit child Builder classes to set the values.

• How will be sustain this in future as and when new *Builder class are introduced?

New builder classes will not work by default. They will require onboarding in up to 2 places

1. Query-Insights plugin to read data type of the new Builder
2. OpenSearch core Builder class to store data type

[deshsidd]

@dzane17 Thanks for the proposal and I too am leaning towards option 1 as we should avoid having this logic in the query-insights plugin.

For option 1 I am concerned regarding the following:

This option is not as intrusive as #1, but there are chances the mapping could be outdated. If we want to keep the mapping up to date, we need to constantly fetch and refresh them.

Please let us know if this would be an issue and how we plan to handle this?

[dzane17]

@deshsidd Actually this concern is related to option 2. There is a time interval between core search execution and generating query shape in Query Insights plugin, so we cannot guarantee that the index mapping for a particular field is present or accurate. This will impact a minute percent of query shapes but is certainly a flaw. Even if we routinely fetch index mappings from Query Insights, I don't see a way to lock the exact mappings at the time of execution.