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

Fix spatial Hilbert to use offset of the minimum range key #2586

Merged
merged 1 commit into from Mar 29, 2018

Conversation

Projects
None yet
2 participants
@echeipesh
Contributor

echeipesh commented Mar 20, 2018

Overview

Spatial Hilbert index incorrectly did not offset the keys being encoded by the minimum key in the KeyBounds of the index. Because Hilbert will attempt to figure out the number of bits required to encode values in the given key range it the following exception can be thrown when KeyBounds provided to the HilbertSpatialKeyIndex constructor does not contain minKey = SptialKey(0, 0)

java.lang.IllegalArgumentException: value doesn't fit

However, it is possible that a layer indexed by HilbertSpatialKeyIndex with minKey > SpatialKey(0,0) would not produce this error when writing if the minimum key was large enough to require the same number of bits as the maximum key. This PR will break reading those layers by changing the mapping from stored record index to SpatialKey instance.

It seems unlikely that this bug would have gone unreported if this is was a prevailing case, however there is a way to fix such layers without re-ingesting them.

IF this PR breaks your layer indexed by HilbertSpatialKeyIndex you can fix it by changing the layer metadata keyIndex.properties.keyBounds.minKey to have col = 0 and row = 0.
This value can be find in the JSON file metadata__<layer_name>__<zoom_level>.json in S3, File, and Hadoop attribute stores.

Before:
screen shot 2018-03-20 at 4 20 11 pm

After:
screen shot 2018-03-20 at 4 20 25 pm

This will not effect the layer KeyBounds as shown in the TileLayerMetadata[K] but only adjust the KeyBounds used by the HilbertSpatialKeyIndex instance, adjusting it to match new reality.

  • docs/CHANGELOG.rst updated, if necessary
    - [ ] docs guides update, if necessary
    - [ ] New user API has useful Scaladoc strings
  • Unit tests added for bug-fix or new feature

@echeipesh echeipesh added the bug label Mar 20, 2018

@echeipesh

This comment has been minimized.

Contributor

echeipesh commented Mar 20, 2018

Because this is basically a glaring bug and evidence is that this is not popular index strategy the intention is to back-port this to 1.2 branch and trigger release of 1.2.2

@echeipesh echeipesh force-pushed the echeipesh:fix/spatial-hilbert-offset branch from 7efbf22 to da11948 Mar 29, 2018

@echeipesh echeipesh force-pushed the echeipesh:fix/spatial-hilbert-offset branch from da11948 to 3e39aa5 Mar 29, 2018

@echeipesh echeipesh merged commit 47e7ee9 into locationtech:master Mar 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment