[
https://issues.apache.org/jira/browse/CALCITE-4294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17599246#comment-17599246
]
Julian Hyde commented on CALCITE-4294:
--------------------------------------
I reviewed. Great, clean code. It's great to see the confusing nested class
{{Geometry.Geom}} disappear from all over the code. Also great that value
strings are clearer (e.g. '\{x:1, y:2.1}' becomes 'POINT (1 2.1)').
A few cosmetic changes to make the code more consistent with the rest of
Calcite:
* Make sure there is a blank line between the javadoc description and the
first @param (for example in {{SpatialTypeUtils.fromWKB}})
* The house style is to convert acronyms to camel-case, so can you change
{{fromWKB}} to {{fromWkb}}. But don't change function names that appear in SQL
via reflection (e.g. "ST_MakePoint").
* In "SpatialTypeUtilsTest" rename "fromEWKT" to "testFromEwkt" and "asEWKT"
to "testAsAwkt" to keep the tradition that test names start with "test".
I saw you added 'round' as a SQL function. That's clever, and makes sense, but
let's give it a different name so that no one confuses it with the built-in
(and standard) ROUND function.
The failure on openjdk17 was a known flakiness issue. Nothing to do with you. I
restarted, and it passed.
As soon as you make those changes, I'll squash and merge into main.
The changes are so clear that I think I will squash into a single commit. I
apologize for that, after I asked you to keep the commits separate. If anyone
needs to do detective work later, the separate commits will still be in the PR.
> Use JTS rather than ESRI as the underlying library for geospatial (ST_)
> functions
> ---------------------------------------------------------------------------------
>
> Key: CALCITE-4294
> URL: https://issues.apache.org/jira/browse/CALCITE-4294
> Project: Calcite
> Issue Type: Bug
> Components: spatial
> Reporter: Julian Hyde
> Assignee: Bertil Chapuis
> Priority: Major
> Fix For: 1.32.0
>
>
> The geospatial functions are currently implemented using the ESRI library. We
> should consider using JTS instead. AT the time we started work on geospatial
> the JTS did not have a suitable license, but this is no longer the case. I
> gather that JTS is a superior library.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
