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

SQL: Switch JDBC driver to libs/geo objects for geo_points #39484

Merged
merged 1 commit into from
Mar 1, 2019

Conversation

imotov
Copy link
Contributor

@imotov imotov commented Feb 27, 2019

Switches JDBC driver to use libs/geo objects when server returns a
geo_point. It also removes separate functions for geo points and geo
shapes, which should simplify developing of geo functions.

Closes #35767

Switches JDBC driver to use libs/geo objects when server returns a
geo_point.

Closes elastic#35767
@imotov imotov added >enhancement :Analytics/Geo Indexing, search aggregations of geo points and shapes :Analytics/SQL SQL querying v8.0.0 labels Feb 27, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@imotov
Copy link
Contributor Author

imotov commented Feb 28, 2019

@elasticmachine run elasticsearch-ci/1

@costin
Copy link
Member

costin commented Mar 1, 2019

If I understand correctly, this PR unifies the aswkt function across geo points and geo shape.
Is it correct to assume going forward that geo functions will support both types as input?

Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM.
I'd like @bpintea opinion as well to make sure the type format is supported on the ODBC side as well.
Besides WKT, are there any plans for other formats (WKB or geojson)?

@bpintea
Copy link
Contributor

bpintea commented Mar 1, 2019

LGTM.
I'd like @bpintea opinion as well to make sure the type format is supported on the ODBC side as well.
Besides WKT, are there any plans for other formats (WKB or geojson)?

@costin I've added a comment to #39204 (comment).

JDBC allows shipping the defined private extensions along with the driver. In ODBC this concept doesn't exist: the interface is defined in the standard and any new extension is described through its characteristics, not by its format.

So, IMO we're free to pick whatever ways to encode the data, for as long as we can export it into a common format, either directly (by maybe using WKT/B or GeoJSON) or indirectly through function transformations.

@imotov
Copy link
Contributor Author

imotov commented Mar 1, 2019

Is it correct to assume going forward that geo functions will support both types as input?

Yes, and it will be easier now since we will have to write only one function. When I started implementing other functions earlier, having 2 sets of functions was a bit problematic.

Besides WKT, are there any plans for other formats (WKB or geojson)?

We are planning to eventually add ST_ functions that will support WKB and maybe other formats. However, I think it would make sense to stick with WKT to transport geo data between server and client for now. I think if we will switch to CBOR as a protocol, we can switch to WKB at that moment. For now, it makes little sense, I think.

@imotov imotov merged commit 078dcf0 into elastic:geosql Mar 1, 2019
@imotov imotov removed the v8.0.0 label Apr 24, 2019
@imotov imotov deleted the issue-35767-libsgeo-geopoint branch May 1, 2020 22:18
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 :Analytics/SQL SQL querying >enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants