>From Suryaa Charan Shivakumar <[email protected]>:

Attention is currently required from: Ian Maxon.

Suryaa Charan Shivakumar has posted comments on this change by Suryaa Charan 
Shivakumar. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21237?usp=email )

Change subject: [ASTERIXDB-3776] Add OGC/PostGIS compatibility layer for 
geospatial functions
......................................................................


Patch Set 2:

(7 comments)

Patchset:

PS2:
Fixed some of these in patchset 2, justified the rest :)


File 
asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/functions/STCollectionExtractDescriptor.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21237/comment/7ff1744a_2d21bc76?usp=email
 :
PS2, Line 35:  PostGIS ST_CollectionExtract: extracts components of the 
requested type from
            :  * a geometry collection, returning them as a homogeneous 
geometry. Type codes
            :  * follow PostGIS
> unless PostGIS is the only implementation of this function let's not refer to 
> it
Yes, I used claude for docs and it does seem like he's doing guerrilla 
marketing. Will remove it in the next patchset


File 
asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/functions/STGeometryNOGCDescriptor.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21237/comment/36924567_22dd4350?usp=email
 :
PS2, Line 28: PostGIS-conformant
> same here
Acknowledged


File 
asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/functions/STGeometryTypeOGCDescriptor.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21237/comment/b756630b_4e199cf0?usp=email
 :
PS2, Line 28:  PostGIS-conformant
> here too
Acknowledged


File 
asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/functions/STLineMergeDescriptor.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21237/comment/da13006d_3d0b9287?usp=email
 :
PS2, Line 47: LineMerger merger = new LineMerger();
> no way to reuse objects here?
I did check try this, as some of our previous work had related comments. My 
understanding is it is single use by design - 
https://locationtech.github.io/jts/javadoc/org/locationtech/jts/operation/linemerge/LineMerger.html
 no way to clear/reset objects or reuse for add method used in 48.


File 
asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/functions/STNumPointsOGCDescriptor.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21237/comment/0e437db0_9afa1a5d?usp=email
 :
PS2, Line 41:         if (!(geometry instanceof LineString)) {
            :             return null;
            :         }
            :         return ((LineString) geometry).getNumPoints();
> this is a method of Geometry isn't it? can't you just use getGeometryType to 
> guard here instead of i […]
In this case and a few others we maybe prone to subclass-blindness, so I felt 
this is a better way to compare, or else the code would have to do multiple 
equals check for LineString and LinearRing (sub class) - 
https://locationtech.github.io/jts/javadoc/org/locationtech/jts/geom/LinearRing.html
Note as of today I believe there is no way type LinearRing would reach here, 
but goal is to write cleaner code considering weird future cases.


File 
asterixdb/asterix-geo/src/main/java/org/apache/asterix/geo/evaluators/functions/STPointNOGCDescriptor.java:

https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21237/comment/1fbb0eba_613f1772?usp=email
 :
PS2, Line 51: LineString line = (LineString) geometry;
            :         if (n < 1 || n > line.getNumPoints()) {
> same here
line.getPointN used in line 55 is a LineString only method :(



--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21237?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://asterix-gerrit.ics.uci.edu/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: Ia6e37080a581292744ddc9020b814927413c16ac
Gerrit-Change-Number: 21237
Gerrit-PatchSet: 2
Gerrit-Owner: Suryaa Charan Shivakumar <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-CC: Ian Maxon <[email protected]>
Gerrit-Attention: Ian Maxon <[email protected]>
Gerrit-Comment-Date: Fri, 15 May 2026 19:46:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Ian Maxon <[email protected]>

Reply via email to