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

WKT polygon format validated with geo_shape, where coerce set to true #35059

Closed
artemabalmasov opened this issue Oct 29, 2018 · 6 comments
Closed
Assignees
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >bug

Comments

@artemabalmasov
Copy link

Good day, trying to integrate twitter location data in WKT format, run into following issue:

System

version: 
{
  "name": "3w66Vhm",
  "cluster_name": "docker-cluster",
  "cluster_uuid": "9dWrPtnVQfWUinPFppNJmA",
  "version": {
    "number": "6.4.2",
    "build_flavor": "default",
    "build_type": "tar",
    "build_hash": "04711c2",
    "build_date": "2018-09-26T13:34:09.098244Z",
    "build_snapshot": false,
    "lucene_version": "7.4.0",
    "minimum_wire_compatibility_version": "5.6.0",
    "minimum_index_compatibility_version": "5.0.0"
  },
  "tagline": "You Know, for Search"
}

jvm: 1.8
plugins: raw docker img from https://www.elastic.co/guide/en/elasticsearch/reference/current/docker.html

Description of the problem including expected versus actual behavior:
Different validation rules for WKT vs own format

Steps to reproduce:

  1. Mapping for example with weak requirements (for twitter) from Update ShapeBuilder and GeoPolygonQueryParser to accept non-closed GeoJSON #11161
PUT /example
{
    "mappings": {
        "doc": {
            "properties": {
                "location": {
                    "type": "geo_shape",
                    "tree": "quadtree",
                    "precision": "100m",
                    "coerce": true
                }
            }
        }
    }
}
  1. Coordinates with usual type works perfectly
POST /example/doc
{
    "location" : {
        "type" : "polygon",
        "coordinates" : [
            [ [114.200488, -8.413208], [114.200488, -8.348076], [114.317869, -8.348076], [114.317869, -8.413208] ]
        ]
    }
}
  1. WKT-format procuses error
POST /example/doc
{
    "location" : "Polygon((114.200488 -8.413208, 114.200488 -8.348076, 114.317869 -8.348076, 114.317869 -8.413208))"
}

Output:

{
   "error": {
      "root_cause": [
         {
            "type": "mapper_parsing_exception",
            "reason": "failed to parse [location]"
         }
      ],
      "type": "mapper_parsing_exception",
      "reason": "failed to parse [location]",
      "caused_by": {
         "type": "illegal_argument_exception",
         "reason": "invalid LinearRing found (coordinates are not closed)"
      }
   },
   "status": 400
}
@mayya-sharipova mayya-sharipova added the :Analytics/Geo Indexing, search aggregations of geo points and shapes label Oct 29, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@imotov imotov self-assigned this Oct 29, 2018
@imotov
Copy link
Contributor

imotov commented Oct 29, 2018

Description of the problem including expected versus actual behavior:
Different validation rules for WKT vs own format

Indeed, the WKT parser ignores the coerce parameter. When a polygon is represented as GeoJSON and it is not closed, we will close it by copying the first coordinate if coerce is set to true. This logic doesn't exist in the WKT parser.

@nknize what do you think? Should we make WKT parser more forgiving if coerce is set to true or should we treat this as a documentation issue?

@nknize
Copy link
Contributor

nknize commented Oct 30, 2018

I think this was an oversight on my part. The spec does require WKT polygons to be closed but coerce was intended to correct those kinds of "simple" issues. So I think we should go ahead and implement coerce support for WKT as well.

imotov added a commit to imotov/elasticsearch that referenced this issue Nov 9, 2018
WKT parser now automatically closes open polygons similar to GeoJSON
parser if coerce flag in mapping is set to true.

Closes to elastic#35059
imotov added a commit to imotov/elasticsearch that referenced this issue Nov 9, 2018
WKT parser now automatically closes open polygons similar to GeoJSON
parser if coerce flag in mapping is set to true.

Closes to elastic#35059
imotov added a commit that referenced this issue Nov 12, 2018
WKT parser now automatically closes open polygons similar to GeoJSON
parser if coerce flag in mapping is set to true.

Closes to #35059
imotov added a commit that referenced this issue Nov 12, 2018
WKT parser now automatically closes open polygons similar to GeoJSON
parser if coerce flag in mapping is set to true.

Closes to #35059
@imotov
Copy link
Contributor

imotov commented Nov 12, 2018

Fixed by #35414

@imotov imotov closed this as completed Nov 12, 2018
pgomulka pushed a commit to pgomulka/elasticsearch that referenced this issue Nov 21, 2018
WKT parser now automatically closes open polygons similar to GeoJSON
parser if coerce flag in mapping is set to true.

Closes to elastic#35059
codebrain added a commit to elastic/elasticsearch-net that referenced this issue May 6, 2019
@russcam
Copy link
Contributor

russcam commented May 7, 2019

@nknize @imotov I don't see the coerce property mentioned in the mapping options for 6.6+: https://www.elastic.co/guide/en/elasticsearch/reference/6.6/geo-shape.html. Should it be added to the docs?

imotov added a commit to imotov/elasticsearch that referenced this issue Jun 18, 2019
Explains the effect of the coerce parameter on the geo_shape field.

Relates elastic#35059
@imotov
Copy link
Contributor

imotov commented Jun 18, 2019

@russcam Thanks for pointing this out. I opened #43340 to fix that .

imotov added a commit that referenced this issue Jun 21, 2019
…3340)

Explains the effect of the coerce parameter on the geo_shape field.

Relates #35059
imotov added a commit that referenced this issue Jun 21, 2019
…3340)

Explains the effect of the coerce parameter on the geo_shape field.

Relates #35059
imotov added a commit that referenced this issue Jun 21, 2019
…3340)

Explains the effect of the coerce parameter on the geo_shape field.

Relates #35059
jkakavas pushed a commit to jkakavas/elasticsearch that referenced this issue Jun 27, 2019
…astic#43340)

Explains the effect of the coerce parameter on the geo_shape field.

Relates elastic#35059
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
Projects
None yet
Development

No branches or pull requests

6 participants