Skip to content

Conversation

@paleolimbot
Copy link
Member

@paleolimbot paleolimbot commented Oct 31, 2025

This PR implements ST_Translate().

--------------------------------------- benchmark 'table=collections_complex': 3 tests --------------------------------------
Name (time in ms)                                                   Median                  Mean             StdDev          
-----------------------------------------------------------------------------------------------------------------------------
test_st_translate[collections_complex-SedonaDBSingleThread]       256.2327 (1.0)        277.4057 (1.0)      40.6925 (1.0)    
test_st_translate[collections_complex-DuckDBSingleThread]       1,277.4913 (4.99)     1,290.4847 (4.65)     46.4146 (1.14)   
test_st_translate[collections_complex-PostGISSingleThread]      6,999.6389 (27.32)    7,026.8960 (25.33)    64.5042 (1.59)   
-----------------------------------------------------------------------------------------------------------------------------

------------------------------------- benchmark 'table=points_simple': 3 tests ------------------------------------
Name (time in ms)                                          Median                Mean              StdDev          
-------------------------------------------------------------------------------------------------------------------
test_st_translate[points_simple-SedonaDBSingleThread]      3.6685 (1.0)        3.8089 (1.0)        1.1013 (1.0)    
test_st_translate[points_simple-DuckDBSingleThread]       10.4048 (2.84)      11.0317 (2.90)       1.2912 (1.17)   
test_st_translate[points_simple-PostGISSingleThread]      36.1355 (9.85)     117.9003 (30.95)    184.4294 (167.46) 
-------------------------------------------------------------------------------------------------------------------

@paleolimbot paleolimbot marked this pull request as ready for review October 31, 2025 21:04
@paleolimbot paleolimbot requested a review from Copilot October 31, 2025 21:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements the ST_Translate() function in Rust, which translates geometry coordinates by fixed X and Y offsets. The implementation leverages the existing CrsTransform infrastructure and includes comprehensive test coverage and benchmarking.

Key changes:

  • New ST_Translate() function implementation with support for 2D and 3D geometries
  • Integration into the function registry with alphabetically sorted entries
  • Test and benchmark coverage for the new functionality

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
rust/sedona-functions/src/st_translate.rs Core implementation of ST_Translate function with geometry transformation logic
rust/sedona-functions/src/register.rs Alphabetically reorganized function registry and added st_translate_udf registration
rust/sedona-functions/src/lib.rs Added st_translate module declaration
rust/sedona-functions/benches/native-functions.rs Added benchmark tests for ST_Translate with Point and LineString geometries
python/sedonadb/tests/functions/test_transforms.py Added parametrized tests for ST_Translate covering edge cases and geometry types
benchmarks/test_transform.py Added performance benchmark comparing SedonaDB and DuckDB implementations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@paleolimbot paleolimbot requested a review from petern48 November 2, 2025 02:39
Copy link
Collaborator

@petern48 petern48 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just suggested some optional nits about testing.

Comment on lines 108 to 111
("POINT Z EMPTY", 1.0, 2.0, "POINT Z (nan nan nan)"),
("POINT (0 1)", 1.0, 2.0, "POINT (1 3)"),
("POINT Z (0 1 2)", 1.0, 2.0, "POINT Z (1 3 2)"),
("LINESTRING EMPTY", 1.0, 2.0, "LINESTRING EMPTY"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: A few cases that aren't tested anywhere (in python or rust). I don't think any would fail, but I think some would be nice to test. Particularly non-point geoms, so it's clear it works on them too. Personally, I often look at tests to see how I can use a function when documentation isn't specific enough.

paleolimbot and others added 2 commits November 2, 2025 20:18
@paleolimbot paleolimbot merged commit 87ff737 into apache:main Nov 3, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants