You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "ajaypadwal73 (via GitHub)" <gi...@apache.org> on 2026/06/20 17:15:18 UTC

[PR] feat: add ST_NumPoints alias for ST_NPoints [sedona-db]

ajaypadwal73 opened a new pull request, #989:
URL: https://github.com/apache/sedona-db/pull/989

   ## Summary
   
   - Adds `ST_NumPoints` as an alias for `ST_NPoints` in `st_points.rs`
   - Adds a test verifying the alias is registered correctly
   
   Closes part of #200.
   
   ## Changes
   
   - `rust/sedona-functions/src/st_points.rs`: added `.with_aliases(vec!["st_numpoints".to_string()])` to `st_npoints_udf()` and a corresponding alias test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add ST_NumPoints alias for ST_NPoints [sedona-db]

Posted by "ajaypadwal73 (via GitHub)" <gi...@apache.org>.
ajaypadwal73 commented on code in PR #989:
URL: https://github.com/apache/sedona-db/pull/989#discussion_r3454907069


##########
python/sedonadb/tests/functions/test_functions.py:
##########
@@ -3772,23 +3772,23 @@ def test_st_numinteriorrings_basic(eng, geom, expected):
     ("geom", "expected"),
     [
         (None, None),
-        ("POINT EMPTY", None),
+        ("POINT EMPTY", 0),
         ("LINESTRING EMPTY", 0),
-        ("POLYGON EMPTY", None),
-        ("MULTIPOINT EMPTY", None),
-        ("MULTILINESTRING EMPTY", None),
-        ("MULTIPOLYGON EMPTY", None),

Review Comment:
   @paleolimbot Done - standalone test_st_numpoints removed and ST_NumPoints assertion added directly into test_st_points alongside ST_NPoints, sharing the same parametrized cases.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add ST_NumPoints alias for ST_NPoints [sedona-db]

Posted by "ajaypadwal73 (via GitHub)" <gi...@apache.org>.
ajaypadwal73 commented on PR #989:
URL: https://github.com/apache/sedona-db/pull/989#issuecomment-4765320869

   @paleolimbot Thanks for the context! Agreed - treating them as exact aliases (consistent with BigQuery) makes more sense than strict PostGIS LineString-only behavior. The old GEOS st_numpoints implementation has been removed and the Python tests updated to reflect the alias semantics.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add ST_NumPoints alias for ST_NPoints [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #989:
URL: https://github.com/apache/sedona-db/pull/989#discussion_r3447871553


##########
docs/reference/sql/st_npoints.qmd:
##########
@@ -29,6 +29,8 @@ kernels:
 
 Returns the total number of coordinate points in the geometry, counting all vertices across all components.
 
+This function also has the alias `ST_NumPoints`.

Review Comment:
   ```suggestion
   Aliases: `ST_NumPoints`.
   ```
   
   Just following the existing pattern we have for that:
   
   https://github.com/apache/sedona-db/blob/ce74a892ea7d8030cc48803c7e8ffa106bfb816f/docs/reference/sql/st_geomfromwkt.qmd#L38



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add ST_NumPoints alias for ST_NPoints [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #989:
URL: https://github.com/apache/sedona-db/pull/989#discussion_r3453286435


##########
python/sedonadb/tests/functions/test_functions.py:
##########
@@ -3772,23 +3772,23 @@ def test_st_numinteriorrings_basic(eng, geom, expected):
     ("geom", "expected"),
     [
         (None, None),
-        ("POINT EMPTY", None),
+        ("POINT EMPTY", 0),
         ("LINESTRING EMPTY", 0),
-        ("POLYGON EMPTY", None),
-        ("MULTIPOINT EMPTY", None),
-        ("MULTILINESTRING EMPTY", None),
-        ("MULTIPOLYGON EMPTY", None),

Review Comment:
   Sorry to bikeshed here, but can we just remove this test and add a second line to the ST_NPoints test? That will ensure that the behaviour stays in sync and any added parameters always apply to both.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: add ST_NumPoints alias for ST_NPoints [sedona-db]

Posted by "ajaypadwal73 (via GitHub)" <gi...@apache.org>.
ajaypadwal73 commented on code in PR #989:
URL: https://github.com/apache/sedona-db/pull/989#discussion_r3448727291


##########
docs/reference/sql/st_npoints.qmd:
##########
@@ -29,6 +29,8 @@ kernels:
 
 Returns the total number of coordinate points in the geometry, counting all vertices across all components.
 
+This function also has the alias `ST_NumPoints`.

Review Comment:
   Done.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(rust/sedona-functions): add ST_NumPoints alias for ST_NPoints [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #989:
URL: https://github.com/apache/sedona-db/pull/989


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org