Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merging mappings with subobjects: false can result in invalid mappings #103497

Closed
felixbarny opened this issue Dec 18, 2023 · 2 comments · Fixed by #103542
Closed

Merging mappings with subobjects: false can result in invalid mappings #103497

felixbarny opened this issue Dec 18, 2023 · 2 comments · Fixed by #103542
Labels
>bug :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch

Comments

@felixbarny
Copy link
Member

Elasticsearch Version

8.11

Installed Plugins

No response

Java Version

bundled

OS Version

macOS 14.2

Problem Description

I found a way to corrupt the mappings using subobjects: false. The reason is that ES is only validating that there must be no object mappers within in an object that's configured with subobjects: false while parsing mappings:

if (objBuilder.subobjects.value() == false && type.equals(NestedObjectMapper.CONTENT_TYPE)) {
throw new MapperParsingException(
"Tried to add nested object ["
+ fieldName
+ "] to object ["
+ objBuilder.name()
+ "] which does not support subobjects"
);
}

However, it's possible to create an invalid mapping using two distinct mapping updates that are valid on their own but invalid when the mappings are merged.

This invalid mapping merge is caught in testing with this assertion that re-parses the merged mappings:

Since assertions are disabled in production, it's possible to create an invalid mapping.

To fix this, we could enable the assertion in production, too. Or, maybe a better way would be to catch the error scenario earlier, during mapping merge.

In these cases, we could check if we're trying to add an ObjectMapper even though subobjects are disabled:

if (mergeIntoMapper == null) {
merged = mergeWithMapper;
} else if (mergeIntoMapper instanceof ObjectMapper objectMapper) {
merged = objectMapper.merge(mergeWithMapper, reason, objectBuilderContext);

Steps to Reproduce

PUT test
{
  "mappings": {
    "properties": {
      "parent": {
        "type": "object",
        "subobjects": false
      }
    }
  }
}

PUT test/_mapping
{
  "properties": {
    "parent.child.grandchild": {
      "type": "keyword"
    }
  }
}
# POST test/_search 503 Service Unavailable
{
  "error": {
    "root_cause": [
      {
        "type": "no_shard_available_action_exception",
        "reason": null
      }
    ],
    "type": "search_phase_execution_exception",
    "reason": "all shards failed",
    "phase": "query",
    "grouped": true,
    "failed_shards": [
      {
        "shard": 0,
        "index": "test",
        "node": null,
        "reason": {
          "type": "no_shard_available_action_exception",
          "reason": null
        }
      }
    ]
  },
  "status": 503
}
# GET test/_mapping 200 OK
{
  "test": {
    "mappings": {
      "properties": {
        "parent": {
          "subobjects": false,
          "properties": {
            "child": {
              "properties": {
                "grandchild": {
                  "type": "keyword"
                }
              }
            }
          }
        }
      }
    }
  }
}

Logs (if relevant)

No response

@felixbarny felixbarny added >bug :Search Foundations/Mapping Index mappings, including merging and defining field types needs:triage Requires assignment of a team area label labels Dec 18, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Dec 18, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@felixbarny
Copy link
Member Author

This will be fixed with #103542

@javanna javanna added Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch and removed Team:Search Meta label for search team labels Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants