Re: [PR] Plumb remaining parameters for ST_Buffer [sedona-db]

[via GitHub]

petern48 commented on code in PR #241:
URL: https://github.com/apache/sedona-db/pull/241#discussion_r2463155943


##########
c/sedona-geos/src/register.rs:
##########
@@ -37,6 +44,7 @@ pub fn scalar_kernels() -> Vec<(&'static str, 
ScalarKernelRef)> {
     vec![
         ("st_area", st_area_impl()),
         ("st_buffer", st_buffer_impl()),
+        ("st_buffer_style", st_buffer_style_impl()),

Review Comment:
   ```suggestion
           ("st_buffer", st_buffer_style_impl()),
   ```
   
   This string here represents the actual string we call in SQL, so we actually 
want this to be `"st_buffer"` and not `"st_buffer_style"`. With this current 
code, we have to instead call it like below. You can verify using the cli (`cd 
sedona-cli/; cargo run`), or using python.
   
   ```sql
   select st_buffer_style(st_geomfromtext('point (0 0)'), 1, '');
   ```
   
   Aside: Skimming through the datafusion code, it looked like `"st_buffer"` 
would be overwritten, since it's just using a map of name -> udf 
[here](https://github.com/apache/datafusion/blob/2a828972d1d4950a688ba5b2525202193bd2cabd/datafusion/core/src/execution/session_state.rs#L1855),
 but when I tried changing the string, both kernels work simultaneously as 
desired. 🤷



-- 
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-unsubscr...@sedona.apache.org

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