Arnab Karmakar has posted comments on this change. ( http://gerrit.cloudera.org:8080/23604 )
Change subject: IMPALA-13263: Add single-argument overload for ST_ConvexHull() ...................................................................... Patch Set 6: (9 comments) http://gerrit.cloudera.org:8080/#/c/23604/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23604/1//COMMIT_MSG@12 PS1, Line 12: Testing: : Added new tes > Adding a Testing header and mention the name and nature of the > tests. http://gerrit.cloudera.org:8080/#/c/23604/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23604/4//COMMIT_MSG@7 PS4, Line 7: > I don't think that mentioning tests is useful - all commits should have cor Done http://gerrit.cloudera.org:8080/#/c/23604/4//COMMIT_MSG@9 PS4, Line 9: h : PostGIS behavior and simplify usage across geometry types. : : Testing: > This looks a bit unnecessarily verbose to me. The single argument overload Done http://gerrit.cloudera.org:8080/#/c/23604/4//COMMIT_MSG@16 PS4, Line 16: > Can you mention that there was no tests at all for ST_ConvexHull()? Done http://gerrit.cloudera.org:8080/#/c/23604/4//COMMIT_MSG@17 PS4, Line 17: 98f5e7592 > nit: I have always seen this written as "geometry types", not geometric. Done http://gerrit.cloudera.org:8080/#/c/23604/4/testdata/workloads/functional-query/queries/QueryTest/geospatial-esri.test File testdata/workloads/functional-query/queries/QueryTest/geospatial-esri.test: http://gerrit.cloudera.org:8080/#/c/23604/4/testdata/workloads/functional-query/queries/QueryTest/geospatial-esri.test@2723 PS4, Line 2723: --- RESULTS > Hmm, this is not related to this change, but I am surprised that the result Ack http://gerrit.cloudera.org:8080/#/c/23604/4/testdata/workloads/functional-query/queries/QueryTest/geospatial-esri.test@2727 PS4, Line 2727: # Collinear points - ESRI returns NULL instead of LINESTRING > Can you add a similar test with other types, e.g. polygon? The point set us Done http://gerrit.cloudera.org:8080/#/c/23604/4/testdata/workloads/functional-query/queries/QueryTest/geospatial-esri.test@2745 PS4, Line 2745: ==== > Can you add a test for a geometry where the convex hull is not valid polygo Done http://gerrit.cloudera.org:8080/#/c/23604/4/testdata/workloads/functional-query/queries/QueryTest/geospatial-esri.test@2756 PS4, Line 2756: 'MULTIPOLYGON (((0 0, 4 0, 3 1, 1 1, 0 0)))' > Can you add a positive test with multiple input geometries? While the ticke Done -- To view, visit http://gerrit.cloudera.org:8080/23604 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb17d98f5e75929ec0143aa16195a84dd6e50796 Gerrit-Change-Number: 23604 Gerrit-PatchSet: 6 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Comment-Date: Thu, 13 Nov 2025 19:33:59 +0000 Gerrit-HasComments: Yes
