fix for duplicate documents sharing the same ID#552
Merged
missinglink merged 1 commit intomasterfrom Mar 28, 2025
Merged
Conversation
Member
Author
|
Example of two versions of the same document being emitted from the document stream: {
_index: 'pelias',
_id: 'whosonfirst:postalcode:454977637',
data: {
name: { default: [Array] },
phrase: { default: [Array] },
parent: {
country: [Array],
country_id: [Array],
country_a: [Array],
country_source: [Array],
county: [Array],
county_id: [Array],
county_a: [Array],
county_source: [Array],
locality: [Array],
locality_id: [Array],
locality_a: [Array],
locality_source: [Array],
macrocounty: [Array],
macrocounty_id: [Array],
macrocounty_a: [Array],
macrocounty_source: [Array],
macroregion: [Array],
macroregion_id: [Array],
macroregion_a: [Array],
macroregion_source: [Array],
region: [Array],
region_id: [Array],
region_a: [Array],
region_source: [Array],
postalcode: [Array],
postalcode_id: [Array],
postalcode_a: [Array],
postalcode_source: [Array]
},
center_point: { lon: -2.79291, lat: 54.053957 },
source: 'whosonfirst',
layer: 'postalcode',
source_id: '454977637',
bounding_box: '{"min_lat":54.053957,"max_lat":54.053957,"min_lon":-2.79291,"max_lon":-2.79291}',
popularity: 9000,
addendum: { concordances: '{"gp:id":"27457509"}' }
}
}{
_index: 'pelias',
_id: 'whosonfirst:postalcode:454977637',
data: {
name: { default: [Array] },
phrase: { default: [Array] },
parent: {
country: [Array],
country_id: [Array],
country_a: [Array],
country_source: [Array],
postalcode: [Array],
postalcode_id: [Array],
postalcode_a: [Array],
postalcode_source: [Array]
},
center_point: { lon: -2.79291, lat: 54.053957 },
source: 'whosonfirst',
layer: 'postalcode',
source_id: '454977637',
bounding_box: '{"min_lat":54.053957,"max_lat":54.053957,"min_lon":-2.79291,"max_lon":-2.79291}',
popularity: 9000,
addendum: { concordances: '{"gp:id":"27457509"}' }
}
} |
missinglink
commented
Mar 27, 2025
|
|
||
| try { | ||
| if (hierarchies && hierarchies.length > 0) { | ||
| hierarchies.forEach(function(hierarchy) { |
Member
Author
There was a problem hiding this comment.
This is the major change
Member
There was a problem hiding this comment.
I verified via a console.log that this code is not called by the PIP service. So it should only affect actual whosonfirst records in Elasticsearch, not admin lookup for any other records like addresses. 👍
a very old PR introduced functionality to generate multiple versions of the same document for each hierarchy present. this results in the document being overridden in elasticsearch with the latter. reverts #87
missinglink
force-pushed
the
fix-multiple-records-per-hierarchy
branch
from
1fdf8e3 to
272ccf4
Compare
orangejulius
approved these changes
Mar 27, 2025
Member
orangejulius
left a comment
orangejulius
left a comment
There was a problem hiding this comment.
Looks good! This is gonna make things cleaner and likely reduce our build sizes slightly as a perk.
Comment on lines
+26
to
+33
| module.exports = (parentRecords) => { | ||
| return (wofRecord) => { | ||
| return wofRecord.hierarchies.map(hierarchy => { | ||
| return Object.values(hierarchy) | ||
| .map(parentId => parentRecords[parentId]) | ||
| .filter(Boolean) | ||
| .filter(r => r.name); | ||
| }); |
Member
There was a problem hiding this comment.
Very nice cleanup here. Modern JS and easier to understand as well.
|
|
||
| try { | ||
| if (hierarchies && hierarchies.length > 0) { | ||
| hierarchies.forEach(function(hierarchy) { |
Member
There was a problem hiding this comment.
I verified via a console.log that this code is not called by the PIP service. So it should only affect actual whosonfirst records in Elasticsearch, not admin lookup for any other records like addresses. 👍
Comment on lines
-7
to
-9
| var isDefined = function(r) { | ||
| return r; | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
a very old PR introduced functionality to generate multiple versions of the same document for each hierarchy present. However, all the documents used the same ID in Elasticsearch. This results in all document being overwritten in Elasticsearch with the document with the last hierarchy.
While there isn't always a strong preferred order, generally we want the first hierarchy.
reverts #87