Remove transform

[nik9000]

Removes the mapping transform feature which when used made debugging very
difficult. Users should transform their documents on the way into
Elasticsearch rather than having Elasticsearch do it.

Closes #12674

[nik9000]

Ping @rjernst for review.

[jpountz]

The change looks good to me, maybe we should document it (unless it already is?) It's a bit weird to me to drop a feature on a minor release. Is it feasible to either deprecate for now and only remove in 3.0 or backport the change to 2.0 since it's not been released yet? cc @clintongormley

[rjernst]

I think the entire "mapping" ScriptContext can be removed?

[rjernst]

Also LGTM (just a couple extra things I think can be removed).

I think 3.0 is the correct place to put this, since we missed the window to get this into 2.0. Heaver mappings cleanup will just have to wait a little longer. :) But let's make sure to deprecate in 2.1.

[nik9000]

I think 3.0 is the correct place to put this, since we missed the window to get this into 2.0. Heaver mappings cleanup will just have to wait a little longer. :) But let's make sure to deprecate in 2.1.

We blasted the documentation in 2.0, IIRC. I'm pretty ambivalent about when it gets removed but @clintongormley wanted to see it gone as soon as we could.

[clintongormley]

Question for me is: what features/improvements does mapping transform block? eg highlighting? are there things we want to get in for 2.x which are blocked?

[mattjanssen]

We just built and deployed a large platform on 2.0 that uses these transforms to extract a geo_point from a geo_shape. We need the geo_point to do distance sorting. We need the geo_shape to do spatial queries.

We assumed it was a safe feature because it hadn't been deprecated. We didn't even notice it wasn't in the latest docs.

We use mongo-connector to ship the data. We don't want to store the lon/lat twice in our DB just to facilitate Elasticsearch. I guess we should rewrite mongo-connector to do transforms?

[lukas-vlcek]

Hmm... I liked mapping transformations.

[jpountz]

Maybe node ingest (#14047) removes the need for transform? So we could remove in 3.0 as @rjernst suggested.

[nik9000]

Maybe node ingest (#14047) removes the need for transform? So we could remove in 3.0 as @rjernst suggested.

Right then. Rebasing onto 3.0.

[nik9000]

Ok - @rjernst, I think this is ready for another round of review when you are ready for it.

[rjernst]

LGTM

[adichad]

I have been using mapping transform native (java) scripts all the way up to 1.7.x in production. never saw a deprecation warning in code or documentation online though. In 2.0 documentation the whole section is not present. the migration tool also did not warn about the transform script reference being present in the type mappings.

Please provide better documentation than throwing a 404 in 2.x documentation at least stating whether this is deprecated or altogether removed in 2.x or not.

In the meanwhile I will look into 2.0 code to see whether I need to incur the effort to move this out now or whether it can wait a month or so.

[rjernst]

@adichad Mapping transforms still exist in 2.x. This PR is to remove them from master (3.0), and a follow up will deprecate them in 2.x.

[adichad]

Thanks @rjernst !
request to reinstate the related documentation for example at

https://www.elastic.co/guide/en/elasticsearch/reference/1.7/mapping-transform.html
https://www.elastic.co/guide/en/elasticsearch/reference/1.7/_get_transformed.html

to also work at

https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-transform.html
https://www.elastic.co/guide/en/elasticsearch/reference/current/_get_transformed.html

and maintain deprecation warnings there?

[nik9000]

Squashed, rebased, and merging to just 3.0.0.

[niemyjski]

Removing this feature really made life tough for us... Do you have any work around for this?

[niemyjski]

cc @nik9000 @rjernst @clintongormley

[niemyjski]

Don't see these changes in here: https://www.elastic.co/guide/en/elasticsearch/reference/5.x/breaking_50_mapping_changes.html We were using this feature like this:
https://github.com/exceptionless/Exceptionless/blob/master/src/Exceptionless.Core/Repositories/Configuration/Indexes/EventIndex.cs#L191-L212

which now completely breaks us.

[jasontedor]

It's on the page that you linked to: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/breaking_50_mapping_changes.html#_source_transform_removed

[niemyjski]

From what I understand there are issues with ingest pipelines around bulk operations??

[nik9000]

The idea is to replace it with the ingest feature. It isn't the same as the
modifications it makes to the source are saved in the source rather than
just used for indexing.

It was a neat feature but ended up breaking more folks than it helped
because it didn't work well with things like highlighting. It made
debugging indexing problems a nightmare.

On Oct 11, 2016 9:15 PM, "Jason Tedor" notifications@github.com wrote:

It's on the page that you linked to: https://www.elastic.co/guide/
en/elasticsearch/reference/5.0/breaking_50_mapping_changes.
html#_source_transform_removed

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13657 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AANLojBAJJ9GLW3QOex07oRQF5yc-usZks5qzDTHgaJpZM4F_5pF
.