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

[GEO] GIS envelope validation #9080

Closed
missinglink opened this issue Dec 28, 2014 · 0 comments · Fixed by #9091
Closed

[GEO] GIS envelope validation #9080

missinglink opened this issue Dec 28, 2014 · 0 comments · Fixed by #9091
Assignees
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >bug good first issue low hanging fruit v1.4.3 v1.5.0 v2.0.0-beta1

Comments

@missinglink
Copy link
Contributor

This is likely related to spatial4j. There is an inconsistency [bug?] in the way the envelope geo-shape type is being validated.

The docs say that the envelope format "consists of coordinates for upper left and lower right points of the shape to represent a bounding rectangle".

However, being human it's pretty easy to screw this up, of the 4 different ways you can specify the co-ordinates; one is correct, two throw validation errors and one bites you every time by not throwing and messing up all your results.

Below I've attached a minimal testcase which outlines the 3 different ways you could screw up specifying the envelope and the status code and error produced for each.

I would expect the last request to error the same as the previous 2 do with a message like "maxX must be >= minX: 1.0 to -1.0"

here's an example of the sort of confusion this causes:

There's actually a bunch more like this in the issue tracker so it's pretty low-hanging-fruit for resolving much frustration, complaining and time-wasting with GIS bbox queries in ES.

#!/bin/bash

################################################
# geo_shape envelope validation
################################################

ES='localhost:9200';

# drop index
curl -XDELETE "$ES/foo?pretty=true" 2>/dev/null;

# create index
curl -XPUT "$ES/foo?pretty=true" -d'
{ 
  "settings": {
    "index": {
      "number_of_shards": 1,
      "number_of_replicas": 0
    }
  },
  "mappings": {
    "bar": {
      "properties": {
        "polys" : {
          "type" : "geo_shape",
          "tree" : "quadtree",
          "tree_levels" : "26"
        }
      }
    }
  }
}';

# index a single polygon, so the index isn't empty
curl -XPOST "$ES/foo/bar/1?pretty=true" -d'
{
  "polys":{
    "type":"Polygon",
    "coordinates":[[
      ["-1","1"],
      ["-1","-1"],
      ["1","-1"],
      ["1","1"],
      ["-1","1"]
    ]]
  }
}';

# top-left / bottom-right (correct)
#200 OK
curl -XPOST "$ES/foo/_search?pretty=true" -d'
{
  "query": {
    "geo_shape": {
      "polys": {
        "shape": {
          "type": "envelope",
          "coordinates": [
            ["-1", "1"],
            ["1", "-1"]
          ]
        }
      }
    }
  }
}';

# bottom-left / top-right (wrong)
#400 InvalidShapeException[maxY must be >= minY: 1.0 to -1.0]
curl -XPOST "$ES/foo/_search?pretty=true" -d'
{
  "query": {
    "geo_shape": {
      "polys": {
        "shape": {
          "type": "envelope",
          "coordinates": [
            ["-1", "-1"],
            ["1", "1"]
          ]
        }
      }
    }
  }
}';

# bottom-right / top-left (wrong)
#400 InvalidShapeException[maxY must be >= minY: 1.0 to -1.0]
curl -XPOST "$ES/foo/_search?pretty=true" -d'
{
  "query": {
    "geo_shape": {
      "polys": {
        "shape": {
          "type": "envelope",
          "coordinates": [
            ["1", "-1"],
            ["-1", "1"]
          ]
        }
      }
    }
  }
}';

# top-right / bottom-left (wrong)
#200 OK
curl -XPOST "$ES/foo/_search?pretty=true" -d'
{
  "query": {
    "geo_shape": {
      "polys": {
        "shape": {
          "type": "envelope",
          "coordinates": [
            ["1", "1"],
            ["-1", "-1"]
          ]
        }
      }
    }
  }
}';
@clintongormley clintongormley added :Analytics/Aggregations Aggregations :Analytics/Geo Indexing, search aggregations of geo points and shapes and removed :Analytics/Aggregations Aggregations labels Dec 29, 2014
@nknize nknize changed the title GIS envelope validation [GEO] GIS envelope validation Dec 29, 2014
nknize added a commit to nknize/elasticsearch that referenced this issue Dec 30, 2014
ShapeBuilder expected coordinates for Envelope types in strict Top-Left, Bottom-Right order. Given that GeoJSON does not enforce coordinate order (as seen in elastic#8672) clients could specify envelope bounds in any order and be compliant with the GeoJSON spec but not the ES ShapeBuilder logic. This change loosens the ShapeBuilder requirements on envelope coordinate order, reordering where necessary.

closes elastic#2544
closes elastic#9067
closes elastic#9079
closes elastic#9080
nknize added a commit that referenced this issue Dec 30, 2014
ShapeBuilder expected coordinates for Envelope types in strict Top-Left, Bottom-Right order. Given that GeoJSON does not enforce coordinate order (as seen in #8672) clients could specify envelope bounds in any order and be compliant with the GeoJSON spec but not the ES ShapeBuilder logic. This change loosens the ShapeBuilder requirements on envelope coordinate order, reordering where necessary.

closes #2544
closes #9067
closes #9079
closes #9080
nknize added a commit that referenced this issue Dec 30, 2014
ShapeBuilder expected coordinates for Envelope types in strict Top-Left, Bottom-Right order. Given that GeoJSON does not enforce coordinate order (as seen in #8672) clients could specify envelope bounds in any order and be compliant with the GeoJSON spec but not the ES ShapeBuilder logic. This change loosens the ShapeBuilder requirements on envelope coordinate order, reordering where necessary.

closes #2544
closes #9067
closes #9079
closes #9080
mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
ShapeBuilder expected coordinates for Envelope types in strict Top-Left, Bottom-Right order. Given that GeoJSON does not enforce coordinate order (as seen in elastic#8672) clients could specify envelope bounds in any order and be compliant with the GeoJSON spec but not the ES ShapeBuilder logic. This change loosens the ShapeBuilder requirements on envelope coordinate order, reordering where necessary.

closes elastic#2544
closes elastic#9067
closes elastic#9079
closes elastic#9080
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 good first issue low hanging fruit v1.4.3 v1.5.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants