-
Notifications
You must be signed in to change notification settings - Fork 26.1k
Adds ST_SIMPLIFY geo spatial function #136309
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
Changes from 17 commits
5dd2dc9
b787a47
4413503
805ab89
ed96b60
e5e3d44
ac2e0be
0e6e1e2
908632b
dd2933e
6ab6bdc
cec7793
4b164d0
8bc10f8
7cc3afe
c117a19
6fcfeb4
a88ba3c
f216fb9
a199465
dfabf12
1aed14a
253619e
9465cb0
2b273fa
90b075d
5d9f936
6c8ed7d
a1e9d29
ed20dd9
465cbec
06f0d74
9c25ce0
5829ea8
f056837
6b5a8fe
9bffd1a
51dc578
a251b62
a64229f
283f546
c14f942
47e211f
89ac8cc
bbfa066
5d4531f
6a99b89
c9408b4
4c957b0
15c06d3
5e443a3
cc55c03
44bf3ba
b57daa7
844b5f8
d838ace
2fd8acb
dc7f70f
556dbca
90d17a7
82bcbec
facd1ee
cd6e24f
a1c27fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| pr: 136309 | ||
| summary: Adds ST_SIMPLIFY geo spatial function | ||
| area: ES|QL | ||
| type: enhancement | ||
| issues: [] | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| Eclipse Distribution License - v 1.0 | ||
|
|
||
| Copyright (c) 2007, Eclipse Foundation, Inc. and its licensors. | ||
|
|
||
| All rights reserved. | ||
|
|
||
| Redistribution and use in source and binary forms, with or without | ||
| modification, are permitted provided that the following conditions are met: | ||
|
|
||
| Redistributions of source code must retain the above copyright notice, this | ||
| list of conditions and the following disclaimer. | ||
|
|
||
| Redistributions in binary form must reproduce the above copyright notice, | ||
| this list of conditions and the following disclaimer in the documentation | ||
| and/or other materials provided with the distribution. | ||
|
|
||
| Neither the name of the Eclipse Foundation, Inc. nor the names of its | ||
| contributors may be used to endorse or promote products derived from this | ||
| software without specific prior written permission. | ||
|
|
||
| THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND | ||
| ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED | ||
| WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE | ||
| DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR | ||
| ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES | ||
| (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; | ||
| LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON | ||
| ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT | ||
| (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS | ||
| SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
||
|
craigtaverner marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,9 @@ | |
| import org.elasticsearch.geometry.utils.GeometryValidator; | ||
| import org.elasticsearch.geometry.utils.WellKnownBinary; | ||
| import org.elasticsearch.geometry.utils.WellKnownText; | ||
| import org.locationtech.jts.io.ParseException; | ||
| import org.locationtech.jts.io.WKTReader; | ||
| import org.locationtech.jts.io.WKTWriter; | ||
|
|
||
| import java.nio.ByteOrder; | ||
|
|
||
|
|
@@ -125,4 +128,20 @@ public String wkbToWkt(BytesRef wkb) { | |
| public Geometry wkbToGeometry(BytesRef wkb) { | ||
| return WellKnownBinary.fromWKB(validator(), false, wkb.bytes, wkb.offset, wkb.length); | ||
| } | ||
|
|
||
| public org.locationtech.jts.geom.Geometry wkbToJtsGeometry(BytesRef wkb) throws ParseException, IllegalArgumentException { | ||
| String wkt = wkbToWkt(wkb); | ||
| if (wkt.startsWith("BBOX")) { | ||
| throw new IllegalArgumentException("Input geometry cannot be a BBOX"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we should convert BBOX to a Polygon here. I think we already do that in a few other places.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll try and do it on my own. Otherwise we agreed on doing it in a separate pr |
||
| } | ||
| WKTReader reader = new WKTReader(); | ||
|
craigtaverner marked this conversation as resolved.
|
||
| return reader.read(wkt); | ||
| } | ||
|
|
||
| public BytesRef jtsGeometryToWkb(org.locationtech.jts.geom.Geometry jtsGeometry) { | ||
| WKTWriter writer = new WKTWriter(); | ||
| String wkt = writer.write(jtsGeometry); | ||
| return wktToWkb(wkt); | ||
|
craigtaverner marked this conversation as resolved.
|
||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3024,3 +3024,135 @@ wkt:keyword |pt:cartesian_point | |
| "POINT(111)" |null | ||
| // end::to_cartesianpoint-str-parse-error-result[] | ||
| ; | ||
|
|
||
| ############################################### | ||
| # Tests for ST_SIMPLIFY | ||
| ############################################### | ||
|
|
||
| stSimplifyMultiRow | ||
| required_capability: st_simplify | ||
|
|
||
| FROM airports | ||
| | SORT name | ||
| | LIMIT 5 | ||
| | EVAL result = st_simplify(TO_GEOSHAPE("POLYGON((-10 -60, 120 -60, 120 60, -10 60, -10 -60))"), 1.0) | ||
| | KEEP name, result | ||
| ; | ||
|
|
||
| name:text | result:geo_shape | ||
| Aba Tenna D. Yilma Int'l | POLYGON ((-10.0 -60.0, -10.0 60.0, 120.0 60.0, 120.0 -60.0, -10.0 -60.0)) | ||
| Abdul Rachman Saleh | POLYGON ((-10.0 -60.0, -10.0 60.0, 120.0 60.0, 120.0 -60.0, -10.0 -60.0)) | ||
| Abidjan Port Bouet | POLYGON ((-10.0 -60.0, -10.0 60.0, 120.0 60.0, 120.0 -60.0, -10.0 -60.0)) | ||
| Abu Dhabi Int'l | POLYGON ((-10.0 -60.0, -10.0 60.0, 120.0 60.0, 120.0 -60.0, -10.0 -60.0)) | ||
| Abuja Int'l | POLYGON ((-10.0 -60.0, -10.0 60.0, 120.0 60.0, 120.0 -60.0, -10.0 -60.0)) | ||
| ; | ||
|
|
||
| stSimplifyMultiRowWithPoints | ||
| required_capability: st_simplify | ||
|
|
||
| FROM airports | ||
| | SORT name | ||
| | LIMIT 5 | ||
| | EVAL result = st_simplify(location, 0.0) | ||
| | KEEP location, result | ||
| ; | ||
|
|
||
| location:geo_point | result:geo_shape | ||
| POINT (41.857756722253 9.61267784753569) | POINT (41.857756722253 9.61267784753569) | ||
|
ncordon marked this conversation as resolved.
Outdated
|
||
| POINT (112.711418617258 -7.92998002840567) | POINT (112.711418617258 -7.92998002840567) | ||
| POINT (-3.93221929167636 5.2543984451492) | POINT (-3.93221929167636 5.2543984451492) | ||
| POINT (54.6463293225558 24.4272271529764) | POINT (54.6463293225558 24.4272271529764) | ||
| POINT (7.27025993974356 9.00437659781094) | POINT (7.27025993974356 9.00437659781094) | ||
| ; | ||
|
|
||
| stSimplifyNoSimplification | ||
| required_capability: st_simplify | ||
|
|
||
| ROW geo_shape = TO_GEOSHAPE("POLYGON((0 0, 1 0.1, 2 0, 2 2, 1 1.9, 0 2, 0 0))") | ||
| | EVAL result = st_simplify(geo_shape, 0.01) | ||
| | KEEP result | ||
| ; | ||
|
|
||
| result:geo_shape | ||
| POLYGON ((0.0 0.0, 0.0 2.0, 1.0 1.9, 2.0 2.0, 2.0 0.0, 1.0 0.1, 0.0 0.0)) | ||
| ; | ||
|
|
||
| stSimplifyWithSimplification | ||
| required_capability: st_simplify | ||
|
|
||
| ROW geo_shape = TO_GEOSHAPE("POLYGON((0 0, 1 0.1, 2 0, 2 2, 1 1.9, 0 2, 0 0))") | ||
| | EVAL result = st_simplify(geo_shape, 0.2) | ||
| | KEEP result | ||
| ; | ||
|
|
||
| result:geo_shape | ||
| POLYGON ((0.0 0.0, 0.0 2.0, 2.0 2.0, 2.0 0.0, 0.0 0.0)) | ||
| ; | ||
|
|
||
| stSimplifyEmptySimplification | ||
| required_capability: st_simplify | ||
|
|
||
| ROW geo_shape = TO_GEOSHAPE("POLYGON((0 0, 1 0.1, 2 0, 2 2, 1 1.9, 0 2, 0 0))") | ||
| | EVAL result = st_simplify(geo_shape, 2.0) | ||
| | KEEP result | ||
| ; | ||
|
|
||
| result:geo_shape | ||
| POLYGON EMPTY | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should return
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ingesting into an index works. As a test, I changed the Do you still think we should change it? What existing Elasticsearch functionality is this inconsistent with? |
||
| ; | ||
|
|
||
| stSimplifyNull | ||
| required_capability: st_simplify | ||
|
|
||
| ROW geo_shape = NULL | ||
| | EVAL result = st_simplify(geo_shape, 2.0) | ||
| | KEEP result | ||
| ; | ||
|
|
||
| result:geo_shape | ||
| NULL | ||
| ; | ||
|
|
||
| stSimplifyCartesianPoint | ||
| required_capability: st_simplify | ||
|
|
||
| # TODO Why cannot we use a latitud outside of -90, 90 when there are tests | ||
| # that use the TO_CARTESIANPOINT that are doing it already? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should definitely be able to work outside that range. Are you saying this fails? That sounds like a bug.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to test this, and the problem is that you have made ST_SIMPLIFY return geo_shape, even if the input type is cartesian. I think ST_SIMPLIFY should return the same type it consumes.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed! |
||
| ROW wkt = ["POINT(97.11 75.53)", "POINT(80.93 72.77)"] | ||
| | MV_EXPAND wkt | ||
| | EVAL pt = TO_CARTESIANPOINT(wkt) | ||
| | EVAL result = st_simplify(pt, 2.0) | ||
| | KEEP result | ||
| ; | ||
|
|
||
| result:geo_shape | ||
| POINT (97.11 75.53) | ||
| POINT (80.93 72.77) | ||
| ; | ||
|
|
||
| stSimplifyCartesianShape | ||
| required_capability: st_simplify | ||
|
|
||
| ROW wkt = ["POINT(97.11 75.53)", "POLYGON((0 0, 1 0.1, 2 0, 2 2, 1 1.9, 0 2, 0 0))"] | ||
| | MV_EXPAND wkt | ||
| | EVAL geom = TO_CARTESIANSHAPE(wkt) | ||
| | EVAL result = st_simplify(geom, 0.2) | ||
| | KEEP result | ||
| ; | ||
|
|
||
| result:geo_shape | ||
| POINT (97.11 75.53) | ||
| POLYGON ((0.0 0.0, 0.0 2.0, 2.0 2.0, 2.0 0.0, 0.0 0.0)) | ||
| ; | ||
|
|
||
| stSimplifyWithIntegerTolerance | ||
| required_capability: st_simplify | ||
|
|
||
| ROW geo_shape = TO_GEOSHAPE("POLYGON((0 0, 1 0.1, 2 0, 2 2, 1 1.9, 0 2, 0 0))") | ||
| | EVAL result = st_simplify(geo_shape, 2) | ||
| | KEEP result | ||
| ; | ||
|
|
||
| result:geo_shape | ||
| POLYGON EMPTY | ||
| ; | ||
Uh oh!
There was an error while loading. Please reload this page.