You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by vvysotskyi <gi...@git.apache.org> on 2017/10/27 12:51:19 UTC
[GitHub] drill pull request #1012: DRILL-5911: Upgrade esri-geometry-api version to 2...
GitHub user vvysotskyi opened a pull request:
https://github.com/apache/drill/pull/1012
DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to avoid depen…
…dency on org.json library
Please see [DRILL-5911](https://issues.apache.org/jira/browse/DRILL-5911) for details.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vvysotskyi/drill DRILL-5911
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/1012.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1012
----
commit 87e892515bfd731a088a3e03ffdc0bef184cf29c
Author: Volodymyr Vysotskyi <vv...@gmail.com>
Date: 2017-10-27T14:45:17Z
DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to avoid dependency on org.json library
----
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the issue:
https://github.com/apache/drill/pull/1012
@vvysotskyi It is a major version change of the dependency, but except for the pom file changes, there are no other changes. It will be good to understand what triggered a major version change (do release notes cover that?) and how it may affect gis plugin. Also, there is another PR on the review related to gis, can this change be combined with the other changes?
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1012
@vrozov, sorry for the misunderstanding. Currently, it is also not supported.
---
[GitHub] drill pull request #1012: DRILL-5911: Upgrade esri-geometry-api version to 2...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/1012
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the issue:
https://github.com/apache/drill/pull/1012
OK. LGTM.
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:
https://github.com/apache/drill/pull/1012
+1, LGTM.
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1012
@vrozov As I understand from the [release notes](https://github.com/Esri/geometry-api-java/releases/tag/v2.0.0), the main reason for the major version change was changing the interface due to the removing org.json dependency. Gis storage plugin provides UDF functions, and they are covered by the existing unit tests, so since all tests are passed this change does not break drill-gis.
I think this change should be merged in the current release, but I don't know when other PR will be merged. Also, another pull request isn't affected by changing esri-geometry-api library, ie this change does not break changes made there.
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Posted by julianhyde <gi...@git.apache.org>.
Github user julianhyde commented on the issue:
https://github.com/apache/drill/pull/1012
If I recall correctly -- we went through this because Calcite uses ESRI -- the only change was to remove org.json. It's a pretty important change because org.json is not compatible with Apache license. And I think org.json data structures were used in the API (e.g. as arguments and method return values) so obsoleting it would be an API change.
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the issue:
https://github.com/apache/drill/pull/1012
@vvysotskyi OK, thanks. Currently, gis unit tests only work with CSV geo-spacial data. It sounds that JSON format is also supported and is not covered by the gis unit tests. Can you confirm if JSON format is supported and if yes, whether it is affected by the major version upgrade or not. In case JSON format is supported, it will be good to cover it in unit tests as well (not as part of this PR).
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1012
@vrozov, If you meant [geojson](http://geojson.org) when saying about JSON format support, then my answer - not, it is not supported yet.
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the issue:
https://github.com/apache/drill/pull/1012
@vvysotskyi I refer to JSON format that ESRI supports by itself.
---