Fix - Requesting _inference_fields when using legacy format causes shard failure

[Samiul-TheSoccerFan]

This PR fixes the shard failure error we get when we request the _inference_fields with legacy semantic_text.

Steps to reproduce:

PUT _inference/sparse_embedding/my-elser-endpoint
{
  "service": "elser",
  "service_settings": {
    "num_allocations": 1,
    "num_threads": 1
  },
  "task_settings": {}
}

PUT my-index-1
{
  "settings": {
    "index.mapping.semantic_text.use_legacy_format": true
  },
  "mappings": {
    "properties": {
      "inference_field": {
        "type": "semantic_text",
        "inference_id": "my-elser-endpoint"
      },
      "source_field": {
        "type": "text",
        "copy_to": "inference_field"
      }
    }
  }
}

PUT my-index-1/_doc/doc1
{
  "inference_field": "test value",
  "source_field": "source value"
}

And then when we search with _inference_fields:

GET my-index-1/_search
{
  "query": {
    "match_all": {}
  },
  "fields": ["_inference_fields"]
}

{
    "error": {
        "root_cause": [
            {
                "type": "null_pointer_exception",
                "reason": "Cannot invoke \"org.elasticsearch.index.mapper.MappedFieldType.isStored()\" because the return value of \"org.elasticsearch.index.query.SearchExecutionContext.getFieldType(String)\" is null"
            }
        ],
        "type": "search_phase_execution_exception",
        "reason": "all shards failed",
        "phase": "query",
        "grouped": true,
        "failed_shards": [
            {
                "shard": 0,
                "index": "my-index-1",
                "node": "EMYfiOmiSiCWfTU1KOHasQ",
                "reason": {
                    "type": "null_pointer_exception",
                    "reason": "Cannot invoke \"org.elasticsearch.index.mapper.MappedFieldType.isStored()\" because the return value of \"org.elasticsearch.index.query.SearchExecutionContext.getFieldType(String)\" is null"
                }
            }
        ],
        "caused_by": {
            "type": "null_pointer_exception",
            "reason": "Cannot invoke \"org.elasticsearch.index.mapper.MappedFieldType.isStored()\" because the return value of \"org.elasticsearch.index.query.SearchExecutionContext.getFieldType(String)\" is null",
            "caused_by": {
                "type": "null_pointer_exception",
                "reason": "Cannot invoke \"org.elasticsearch.index.mapper.MappedFieldType.isStored()\" because the return value of \"org.elasticsearch.index.query.SearchExecutionContext.getFieldType(String)\" is null"
            }
        }
    },
    "status": 500
}

[elasticsearchmachine]

Hi @Samiul-TheSoccerFan, I've created a changelog YAML for you.

[elasticsearchmachine]

Pinging @elastic/search-eng (Team:SearchOrg)

[elasticsearchmachine]

Pinging @elastic/search-relevance (Team:Search - Relevance)

[kderusso]

Great start @Samiul-TheSoccerFan !

@Mikep86 has already given all of the feedback I would have, once you address those comments I think this will be in a good place.

[jimczi]

Sorry, I misread the change, it is related to the inference fields.
There's something I don't understand here though, the issue comes from this code:

searchExecutionContext.isMetadataField(fieldAndFormat.field) == false || searchExecutionContext.getFieldType(fieldAndFormat.field).isStored() == false

The inference fields must be detected as a non-metadata field since the legacy format is used, meaning that we should never reach the getFieldType part. The issue here it seems is that isMetadataField returns true but then the field type is null. That should never happen since all metadata fields are mapped.

[jimczi]

Ok I think I understand now. MapperService#isMetadataField returns whether the field might be a metadata field in the general context instead of checking the actual value of the field in the mapping. So instead of doing:

public boolean isMetadataField(String field) {
        return mapperRegistry.getMetadataMapperParsers(indexVersionCreated).containsKey(field);
    }

We should do something like:

public boolean isMetadataField(String field) {
        var mapper = mappingLookup().getMapper(field);
        return mapper != null && mapper instanceof MetadataFieldMapper;
}

The _inference_fields is a metadata field only on indices where index.mapping.semantic_text.use_legacy_format and current check on MapperService#isMetadataField doesn't account for that.

[Mikep86]

Looks much better 🙌 ! I left one comment that we should address as I think not_exists will return true if any component of the queried path does not exist, meaning that the test could pass if no hits are returned.

[Mikep86]

LGTM!

[kderusso]

Nice work, thanks for making all of the iterations!

[Mikep86]

@Samiul-TheSoccerFan Looks like these changes broke testIsMetadataField, can you take a look?

[Mikep86]

Let's fix the broken test before merging

[Mikep86]

Good approach to fix the failing test. I added some comments about how we can clean it up.

[Mikep86]

LGTM, thanks for the iterations!

[Samiul-TheSoccerFan]

@jimczi Can you please take a look at this PR again and let us know if you have any concerns or suggestions.

[jimczi]

LGTM

[elasticsearchmachine]

💚 Backport successful

Status	Branch	Result
✅	9.0
✅	8.18

[Samiul-TheSoccerFan]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Questions ?

Please refer to the Backport tool documentation