Add new point field. #53804
Add new point field. #53804
Conversation
iverase
added
>feature
:Analytics/Geo
|
Pinging @elastic/es-analytics-geo (:Analytics/Geo) |
|
@elasticmachine test this please |
...lugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/CartesianPoint.java
Outdated
Show resolved
Hide resolved
| return builder.startObject().field("x", x).field("y", y).endObject(); | ||
| } | ||
|
|
||
| public static CartesianPoint parsePoint(XContentParser parser, CartesianPoint point) |
There was a problem hiding this comment.
I know this is largely a mirror of GeoPoint but I'm wondering if we can achieve this using ObjectParser or ConstructingObjectParser? (something we've been meaning to address w/ GeoPoint as well). I'm not sure how that works with the parsing leniency we have on Point and GeoPoint (e.g., array vs comma delimited string). Curious if @nik9000 or @talevy have some ideas. Maybe that can be done in a followup PR.
...gin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java
Show resolved
Hide resolved
...gin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java
Outdated
Show resolved
Hide resolved
...gin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java
Outdated
Show resolved
Hide resolved
| if (fieldType().stored()) { | ||
| context.doc().add(new StoredField(fieldType().name(), point.toString())); | ||
| } | ||
| if (fieldType.hasDocValues()) { |
There was a problem hiding this comment.
The more I think about this the more I think it's best we disable doc values for this PR and give it some more thought in a future PR. That's fine considering we weren't planning to add cartesian aggregation support until a future version anyway.
There was a problem hiding this comment.
The reason I added doc values was to be able to execute IndexOrDocValuesQueries. This doc values are generated at Lucene level and my feeling is that it should not be a problem for backwards compatibility as they are already part of Lucene distribution.
...tial/src/main/java/org/elasticsearch/xpack/spatial/index/query/ShapeQueryPointProcessor.java
Outdated
Show resolved
Hide resolved
...tial/src/main/java/org/elasticsearch/xpack/spatial/index/query/ShapeQueryPointProcessor.java
Outdated
Show resolved
Hide resolved
...patial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java
Outdated
Show resolved
Hide resolved
...patial/src/test/java/org/elasticsearch/xpack/spatial/index/query/ShapeQueryBuilderTests.java
Show resolved
Hide resolved
|
I know we were originally planning to have this in time for 7.7 feature freeze. But given that it's a highly visible (and extremely useful) feature I'd rather it slide to 7.8 and make sure we've thought through the doc values than risk rushing it and having to support bwc. |
| try { | ||
| CartesianPoint.assertZValue(ignoreZValue, Float.parseFloat(vals[2].trim())); | ||
| } catch (NumberFormatException ex) { | ||
| throw new ElasticsearchParseException("[{}]] must be a number", Y_FIELD.getPreferredName()); |
There was a problem hiding this comment.
@iverase, Y_FIELD.getPreferredName() should be Z_FIELD.getPreferredName()?
Relates: #4718, elastic/elasticsearch#53804 This commit adds a Point property to map the new point data type. A CartesianPoint type is introduced to model points.
Relates: #4718, elastic/elasticsearch#53804 This commit adds a Point property to map the new point data type. A CartesianPoint type is introduced to model points.
Relates: #4718, elastic/elasticsearch#53804 This commit adds a Point property to map the new point data type. A CartesianPoint type is introduced to model points.
Relates: #4718, elastic/elasticsearch#53804 This commit adds a Point property to map the new point data type. A CartesianPoint type is introduced to model points.
Relates: #4718, elastic/elasticsearch#53804 This commit adds a Point property to map the new point data type. A CartesianPoint type is introduced to model points.
Relates: #4718, elastic/elasticsearch#53804 This commit adds a Point property to map the new point data type. A CartesianPoint type is introduced to model points.
Relates: #4718, elastic/elasticsearch#53804 This commit adds a Point property to map the new point data type. A CartesianPoint type is introduced to model points. Co-authored-by: Russ Cam <[email protected]>
This commit adds a new point field that is able to index arbitrary pair of values (x/y) in the cartesian space. It only supports filtering using shape queries at the moment.