Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SEDONA-498] Add Spheroidal ST_Buffer #1251

Merged
merged 60 commits into from
Mar 4, 2024

Conversation

prantogg
Copy link
Contributor

@prantogg prantogg commented Feb 22, 2024

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

  • Adds Spheroidal support for ST_Buffer. Functions as follows,

    • Uses ST_BestSRID to determine the optimal SRID based on the geometry's extent and location.
    • Performs ST_ShiftLongitude on geometries crossing the IDL. ST_CrossesDateLine is used to determine if geometry crosses the IDL.
    • Transforms the geometry to the selected SRID for buffering, defaulting to WGS 84 (SRID 4326) if no SRID is set.
    • Applies standard planar buffer operation in the chosen coordinate system.
    • Transforms the buffered geometry back to its original SRID, or to WGS 84 if the original SRID was not set.
    • Normalize longitudes of resultant geometry back to [-180, 180].
    • Behaves similar to PostGIS's ST_Buffer(Geography).
  • Updates ST_BestSRID as follows,

    • Handle geometries crossing the dateline. Performs ST_ShiftLongitude on geometries crossing the IDL determined by ST_CrossesDateLine
    • Update UTM zone calculation for geometries with shifted longitudes.

How was this patch tested?

  • Passes new and existing tests

Did this PR include necessary documentation updates?

@prantogg prantogg marked this pull request as ready for review February 22, 2024 04:50
docs/api/sql/Function.md Show resolved Hide resolved
@prantogg prantogg marked this pull request as ready for review February 29, 2024 22:54
@jiayuasu jiayuasu added this to the sedona-1.6.0 milestone Mar 1, 2024
min = Math.min(points[i].getX(), min);
}
return min;
}

public static double xMax(Geometry geometry) {
public static double xMax (Geometry geometry){
Copy link
Member

Choose a reason for hiding this comment

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

Why does this PR fix the format of every single function of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My IDE (IntelliJ) automatically reformatted the code style when resolving merge conflicts. Should I rollback these format changes?

Copy link
Member

Choose a reason for hiding this comment

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

@prantogg I would prefer that if you can roll back the format changes.

@@ -571,10 +590,13 @@ Format:
```
ST_Buffer (A: Geometry, buffer: Double, bufferStyleParameters: String [Optional])
```
```
ST_Buffer (A: Geometry, buffer: Double, bufferStyleParameters: String [Optional], useSperoidal: Boolean)
Copy link
Member

Choose a reason for hiding this comment

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

UseSpheroid parameter should be the 3rd param.

docs/api/snowflake/vector-data/Function.md Outdated Show resolved Hide resolved
val polygonDf = sparkSession.sql("select ST_GeomFromWKT(polygontable._c0) as countyshape from polygontable")
polygonDf.createOrReplaceTempView("polygondf")
val functionDf = sparkSession.sql("select ST_Buffer(polygondf.countyshape, 1, true) from polygondf")
assert(functionDf.count() > 0);
Copy link
Member

Choose a reason for hiding this comment

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

Please add a test that uses ST_Buffer with all 4 params

@prantogg prantogg requested a review from jiayuasu March 3, 2024 18:37
min = Math.min(points[i].getX(), min);
}
return min;
}

public static double xMax(Geometry geometry) {
public static double xMax (Geometry geometry){
Copy link
Member

Choose a reason for hiding this comment

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

@prantogg I would prefer that if you can roll back the format changes.

@prantogg prantogg requested a review from jiayuasu March 4, 2024 03:04
The optional third parameter, `useSpheroid`, controls the mode of buffer calculation.

- Planar Buffering (default): When `useSpheroid` is false, `ST_Buffer` performs standard planar buffering based on the provided parameters.
- Spheroidal Buffering:
Copy link
Member

Choose a reason for hiding this comment

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

One last thing. Please add that, when useSpheroid is set to true, the unit of the buffer is meter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@prantogg prantogg requested a review from jiayuasu March 4, 2024 03:45
@jiayuasu jiayuasu merged commit 3f4697a into apache:master Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

st_buffer,ST_DWithin
2 participants