Skip to content

Fix dynamic mapping update generation. #27467

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

Merged

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Nov 21, 2017

When a field is not mapped, Elasticsearch tries to generate a mapping update
from the parsed document. Some documents can introduce corner-cases, for
instance in the event of a multi-valued field whose values would be mapped to
different field types if they were supplied on their own, see for instance:

PUT index/doc/1
{
  "foo": ["2017-11-10T02:00:01.247Z","bar"]
}

In that case, dynamic mappings want to map the first value as a date field
and the second one as a text field. This currently throws an exception,
which is expected, but the wrong one since it throws a class_cast_exception
(which triggers a HTTP 5xx code) when it should throw an
illegal_argument_exception (HTTP 4xx).

When a field is not mapped, Elasticsearch tries to generate a mapping update
from the parsed document. Some documents can introduce corner-cases, for
instance in the event of a multi-valued field whose values would be mapped to
different field types if they were supplied on their own, see for instance:

```
PUT index/doc/1
{
  "foo": ["2017-11-10T02:00:01.247Z","bar"]
}
```

In that case, dynamic mappings want to map the first value as a `date` field
and the second one as a `text` field. This currently throws an exception,
which is expected, but the wrong one since it throws a `class_cast_exception`
(which triggers a HTTP 5xx code) when it should throw an
`illegal_argument_exception` (HTTP 4xx).
@jpountz jpountz added :Search Foundations/Mapping Index mappings, including merging and defining field types >bug v5.6.5 v6.0.1 v6.1.0 v7.0.0 labels Nov 21, 2017
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andrewvc
Copy link
Contributor

@jpountz exactly which 4xx error will this wind up returning?

@jasontedor
Copy link
Member

@andrewvc A generic illegal argument exception is treated as a generic bad request so 400.

@andrewvc
Copy link
Contributor

@jasontedor hmmm, so that means these events are un-indexable (LS will send them to the DLQ). That seems right to me.

@jpountz jpountz merged commit 6ac7990 into elastic:master Nov 21, 2017
@jpountz jpountz deleted the fix/class_cast_exception_dyn_mappings branch November 21, 2017 14:31
jpountz added a commit that referenced this pull request Nov 21, 2017
When a field is not mapped, Elasticsearch tries to generate a mapping update
from the parsed document. Some documents can introduce corner-cases, for
instance in the event of a multi-valued field whose values would be mapped to
different field types if they were supplied on their own, see for instance:

```
PUT index/doc/1
{
  "foo": ["2017-11-10T02:00:01.247Z","bar"]
}
```

In that case, dynamic mappings want to map the first value as a `date` field
and the second one as a `text` field. This currently throws an exception,
which is expected, but the wrong one since it throws a `class_cast_exception`
(which triggers a HTTP 5xx code) when it should throw an
`illegal_argument_exception` (HTTP 4xx).
jpountz added a commit that referenced this pull request Nov 21, 2017
When a field is not mapped, Elasticsearch tries to generate a mapping update
from the parsed document. Some documents can introduce corner-cases, for
instance in the event of a multi-valued field whose values would be mapped to
different field types if they were supplied on their own, see for instance:

```
PUT index/doc/1
{
  "foo": ["2017-11-10T02:00:01.247Z","bar"]
}
```

In that case, dynamic mappings want to map the first value as a `date` field
and the second one as a `text` field. This currently throws an exception,
which is expected, but the wrong one since it throws a `class_cast_exception`
(which triggers a HTTP 5xx code) when it should throw an
`illegal_argument_exception` (HTTP 4xx).
jpountz added a commit that referenced this pull request Nov 21, 2017
When a field is not mapped, Elasticsearch tries to generate a mapping update
from the parsed document. Some documents can introduce corner-cases, for
instance in the event of a multi-valued field whose values would be mapped to
different field types if they were supplied on their own, see for instance:

```
PUT index/doc/1
{
  "foo": ["2017-11-10T02:00:01.247Z","bar"]
}
```

In that case, dynamic mappings want to map the first value as a `date` field
and the second one as a `text` field. This currently throws an exception,
which is expected, but the wrong one since it throws a `class_cast_exception`
(which triggers a HTTP 5xx code) when it should throw an
`illegal_argument_exception` (HTTP 4xx).
jasontedor added a commit to olcbean/elasticsearch that referenced this pull request Nov 21, 2017
* master: (41 commits)
  [Test] Fix AggregationsTests#testFromXContentWithRandomFields
  [DOC] Fix mathematical representation on interval (range) (elastic#27450)
  Update version check for CCS optional remote clusters
  Bump BWC version to 6.1.0 for elastic#27469
  Adapt rest test BWC version after backport
  Fix dynamic mapping update generation. (elastic#27467)
  Use the primary_term field to identify parent documents (elastic#27469)
  Move composite aggregation to core (elastic#27474)
  Fix test BWC version after backport
  Protect shard splitting from illegal target shards (elastic#27468)
  Cross Cluster Search: make remote clusters optional (elastic#27182)
  [Docs] Fix broken bulleted lists (elastic#27470)
  Move resync request serialization assertion
  Fix resync request serialization
  Fix issue where pages aren't released (elastic#27459)
  Add YAML REST tests for filters bucket agg (elastic#27128)
  Remove tcp profile from low level nio channel (elastic#27441)
  [TEST] Fix `GeoShapeQueryTests#testPointsOnly` failure
  Transition transport apis to use void listeners (elastic#27440)
  AwaitsFix GeoShapeQueryTests#testPointsOnly elastic#27454
  ...
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
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 v5.6.5 v6.0.1 v6.1.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants