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

Remove _type field #95

Merged
merged 1 commit into from
Sep 13, 2019
Merged

Remove _type field #95

merged 1 commit into from
Sep 13, 2019

Conversation

orangejulius
Copy link
Member

@orangejulius orangejulius commented May 16, 2018

Currently, we create numerous Elasticsearch types, corresponding to different layers. All the types are identical, so they don't really serve any value.

In Elasticsearch 6 mapping types will go away. The sooner we can remove our minimal usage of types, the easier that transition will be.

Its possible that this will give us a performance benefit right away, although it probably won't. It will simplify our code a bit though!

Note: This PR can be merged right away. Later on, we can merge pelias/schema#293 to enforce a single type everywhere.

Connects pelias/pelias#719

@orangejulius
Copy link
Member Author

While it would be really nice to merge this now and cut our schema size down considerably, we should wait until pelias/pelias#461 is complete so that we can set the document name to the new default (_doc). It isn't a valid value in ES2, so we would have to change it again later.

@orangejulius
Copy link
Member Author

This PR is old, but we should still merge it eventually. However, before doing so, we will need to fix pelias/pelias#672 (probably by storing the Pelias GID in the Elasticsearch _id_ field).

Otherwise, removing our use of the Elasticsearch _type field would make that problem even worse.

@orangejulius
Copy link
Member Author

I just did some refreshing of this PR. It's good to go, however we should wait a bit until #121 has had a chance to make its way out.

orangejulius added a commit to pelias/whosonfirst that referenced this pull request Sep 13, 2019
orangejulius added a commit to pelias/openaddresses that referenced this pull request Sep 13, 2019
This should have been done as part of
pelias/model#95

Looks like maybe greenkeeper missed it?
orangejulius added a commit to pelias/openaddresses that referenced this pull request Sep 13, 2019
orangejulius added a commit to pelias/openaddresses that referenced this pull request Sep 13, 2019
BREAKING CHANGE: This change will affect the functional tests on most
importers

Currently, we create numerous elasticsearch types, corresponding to
different layers. All the types are identical, so they don't really
serve any value.

In Elasticsearch 6 [mapping types will go away](https://www.elastic.co/guide/en/elasticsearch/reference/6.2/removal-of-types.html).
The sooner we can remove our minimal usage of types, the easier that
transition will be.

Connects pelias/pelias#719
@orangejulius
Copy link
Member Author

Just did some final testing of this PR. It appears to work great, however it's a breaking change as far as the importers are concerned, as many of their functional tests are expecting certain values for the _type field, which will change with this PR.

The commit has been updated to be a breaking change, and so this is finally good to merge after over a year!

@orangejulius orangejulius merged commit 9822268 into master Sep 13, 2019
@orangejulius orangejulius deleted the remove-types branch September 13, 2019 16:25
orangejulius added a commit to pelias/openstreetmap that referenced this pull request Sep 13, 2019
After pelias/model#95, and in anticipation of
Elasticsearch 6 support, all use of the `_type` property on
Elasticsearch documents has to be avoided.

This updates the end to end test expectations with the new single
document type, and updates some lingering references in the code to the
`_type` field.
orangejulius added a commit to pelias/openstreetmap that referenced this pull request Sep 13, 2019
After pelias/model#95, and in anticipation of
Elasticsearch 6 support, all use of the `_type` property on
Elasticsearch documents has to be avoided.

This updates the end to end test expectations with the new single
document type, and updates some lingering references in the code to the
`_type` field.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant