You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org> on 2026/01/14 20:20:05 UTC

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Csaba Ringhofer has uploaded this change for review. ( http://gerrit.cloudera.org:8080/23874


Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................

IMPALA-14629: Implement st_point(double,double) in c++

Adds native implementation for the common 2d constructor while
keeps the more complex st_point(string) in Java. Tried using
boost's WKT parser but making it work with >2d geometries
seemed non-trivial.

Also removes the following overloads:
st_point(double,double,double)
st_point(double,double,double,double)
st_pointz(double,double,double,double)
These conflict with overlaods that have different semantics in
PostGis, see HIVE-29395 for details.

Testing:
- no test changes are needed - the remaining constructors
  are covered while the removed ones were not used at all

Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
---
M be/src/exprs/geo/geospatial-functions-ir.cc
M be/src/exprs/geo/geospatial-functions.h
M be/src/exprs/geo/shape-format.h
M fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java
4 files changed, 54 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/23874/1
-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/21397/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jan 2026 20:39:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 3: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/12937/


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jan 2026 19:22:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/12948/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Jan 2026 08:13:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/12964/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Jan 2026 09:52:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 6: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Jan 2026 15:22:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 2: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jan 2026 20:48:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................

IMPALA-14629: Implement st_point(double,double) in c++

Adds native implementation for the common 2d constructor while
keeps the more complex st_point(string) in Java. Tried using
boost's WKT parser but making it work with >2d geometries
seemed non-trivial.

Also removes the following overloads:
st_point(double,double,double)
st_point(double,double,double,double)
st_pointz(double,double,double,double)
These conflict with overloads that have different semantics in
PostGis, see HIVE-29395 for details.

Testing:
- no test changes are needed - the remaining constructors
  are covered while the removed ones were not used at all

Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Reviewed-on: http://gerrit.cloudera.org:8080/23874
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exprs/geo/geospatial-functions-ir.cc
M be/src/exprs/geo/geospatial-functions.h
M be/src/exprs/geo/shape-format.h
M fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java
M testdata/workloads/functional-query/queries/QueryTest/geospatial-esri-planner.test
5 files changed, 58 insertions(+), 6 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/21415/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Jan 2026 09:10:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Daniel Vanko (Code Review)" <ge...@cloudera.org>.
Daniel Vanko has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 5: Code-Review+1

> Patch Set 5:
> 
> PS4 and PS5 change geospatial-esri-planner.test to conform  with changes in https://gerrit.cloudera.org/#/c/23719/ and https://gerrit.cloudera.org/#/c/23864/ The broken test fixed by PS4 was assuming that an expression is considered costly due to the Java functions involved, but this was no longer true due to moving to native function with st_point()

Could you update the commit message accordingly?


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Sun, 18 Jan 2026 18:33:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jan 2026 13:58:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 5: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Jan 2026 13:53:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Daniel Vanko (Code Review)" <ge...@cloudera.org>.
Daniel Vanko has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 2: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jan 2026 10:35:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Peter Rozsa, Daniel Vanko, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/23874

to look at the new patch set (#4).

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................

IMPALA-14629: Implement st_point(double,double) in c++

Adds native implementation for the common 2d constructor while
keeps the more complex st_point(string) in Java. Tried using
boost's WKT parser but making it work with >2d geometries
seemed non-trivial.

Also removes the following overloads:
st_point(double,double,double)
st_point(double,double,double,double)
st_pointz(double,double,double,double)
These conflict with overloads that have different semantics in
PostGis, see HIVE-29395 for details.

Testing:
- no test changes are needed - the remaining constructors
  are covered while the removed ones were not used at all

Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
---
M be/src/exprs/geo/geospatial-functions-ir.cc
M be/src/exprs/geo/geospatial-functions.h
M be/src/exprs/geo/shape-format.h
M fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java
M testdata/workloads/functional-query/queries/QueryTest/geospatial-esri-planner.test
5 files changed, 56 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/23874/4
-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/23874

to look at the new patch set (#2).

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................

IMPALA-14629: Implement st_point(double,double) in c++

Adds native implementation for the common 2d constructor while
keeps the more complex st_point(string) in Java. Tried using
boost's WKT parser but making it work with >2d geometries
seemed non-trivial.

Also removes the following overloads:
st_point(double,double,double)
st_point(double,double,double,double)
st_pointz(double,double,double,double)
These conflict with overloads that have different semantics in
PostGis, see HIVE-29395 for details.

Testing:
- no test changes are needed - the remaining constructors
  are covered while the removed ones were not used at all

Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
---
M be/src/exprs/geo/geospatial-functions-ir.cc
M be/src/exprs/geo/geospatial-functions.h
M be/src/exprs/geo/shape-format.h
M fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java
4 files changed, 54 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/23874/2
-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/21416/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Jan 2026 09:20:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/12937/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jan 2026 14:25:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jan 2026 14:25:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Peter Rozsa, Daniel Vanko, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/23874

to look at the new patch set (#5).

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................

IMPALA-14629: Implement st_point(double,double) in c++

Adds native implementation for the common 2d constructor while
keeps the more complex st_point(string) in Java. Tried using
boost's WKT parser but making it work with >2d geometries
seemed non-trivial.

Also removes the following overloads:
st_point(double,double,double)
st_point(double,double,double,double)
st_pointz(double,double,double,double)
These conflict with overloads that have different semantics in
PostGis, see HIVE-29395 for details.

Testing:
- no test changes are needed - the remaining constructors
  are covered while the removed ones were not used at all

Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
---
M be/src/exprs/geo/geospatial-functions-ir.cc
M be/src/exprs/geo/geospatial-functions.h
M be/src/exprs/geo/shape-format.h
M fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java
M testdata/workloads/functional-query/queries/QueryTest/geospatial-esri-planner.test
5 files changed, 58 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/23874/5
-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 5:

PS4 and PS5 change geospatial-esri-planner.test to conform  with changes in https://gerrit.cloudera.org/#/c/23719/ and https://gerrit.cloudera.org/#/c/23864/ The broken test fixed by PS4 was assuming that an expression is considered costly due to the Java functions involved, but this was no longer true due to moving to native function with st_point()


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Jan 2026 08:13:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Jan 2026 09:51:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-14629: Implement st point(double,double) in c++

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23874 )

Change subject: IMPALA-14629: Implement st_point(double,double) in c++
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/23874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I927413f92cf4d4e9a995f7024de0ec2e3b584b6d
Gerrit-Change-Number: 23874
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Vanko <dv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Jan 2026 09:52:19 +0000
Gerrit-HasComments: No