Skip to content

Conversation

@imotov
Copy link
Contributor

@imotov imotov commented Apr 6, 2018

Fixes a possible overflow error that geohashes longer than 12 characters
can cause during parsing.

Fixes #24616

Fixes a possible overflow error that geohashes longer than 12 character
can cause during parsing.
@imotov imotov added >bug review :Analytics/Geo Indexing, search aggregations of geo points and shapes v7.0.0 v6.3.0 labels Apr 6, 2018
@imotov imotov requested a review from jpountz April 6, 2018 20:07
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@imotov imotov requested a review from nknize April 6, 2018 20:07
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

It looks good but let's wait for @nknize to have a second look. Let's also clarify in the docs that geohashes with more than 12 levels don't provide additional accuracy (and reassure them that it's very likely enough for them)?

@imotov
Copy link
Contributor Author

imotov commented Apr 10, 2018

@jpountz thanks for the review! I added a note to the documentation.

Copy link
Contributor

@nknize nknize left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks @imotov

@nknize nknize removed the review label Apr 16, 2018
@imotov imotov merged commit e334baf into elastic:master Apr 16, 2018
imotov added a commit that referenced this pull request Apr 16, 2018
Fixes a possible overflow error that geohashes longer than 12 characters
can cause during parsing.

Fixes #24616
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 16, 2018
* master:
  Remove PipelineExecutionService#executeIndexRequest (elastic#29537)
  Fix overflow error in parsing of long geohashes (elastic#29418)
  Remove unused index.ttl.disable_purge setting (elastic#29527)
  FullClusterRestartIT.testRecovery should wait for all initializing shards
  Build: Fail if any libs depend on non-core libs (elastic#29336)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 17, 2018
* master: (96 commits)
  TEST: Mute testEnsureWeReconnect
  Mute full cluster restart test recovery test
  REST high-level client: add support for Indices Update Settings API [take 2] (elastic#29327)
  Plugins: Fix native controller confirmation for non-meta plugin (elastic#29434)
  Remove PipelineExecutionService#executeIndexRequest (elastic#29537)
  Fix overflow error in parsing of long geohashes (elastic#29418)
  Remove unused index.ttl.disable_purge setting (elastic#29527)
  FullClusterRestartIT.testRecovery should wait for all initializing shards
  Build: Fail if any libs depend on non-core libs (elastic#29336)
  [TEST] REST client request without leading '/' (elastic#29471)
  Using ObjectParser in UpdateRequest (elastic#29293)
  Prevent accidental changes of default values (elastic#29528)
  [Docs] Add definitions to glossary  (elastic#29127)
  Avoid self-deadlock in the translog (elastic#29520)
  Minor cleanup in NodeInfo.groovy
  Lazy configure build tasks that require older JDKs (elastic#29519)
  Simplify snapshot check in root build file
  Make NodeInfo#nodeVersion strongly-typed as Version (elastic#29515)
  Enable license header exclusions (elastic#29379)
  Use proper Java version for BWC builds (elastic#29493)
  ...
@imotov imotov deleted the issue-24616-geohash-overflow branch May 1, 2020 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Geo Indexing, search aggregations of geo points and shapes >bug v6.3.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants