[GH-3061] Add geography constructors to SedonaFlink#3062
Open
jiayuasu wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR wires Sedona’s existing common-module Geography constructor/conversion APIs into SedonaFlink so they are usable from the Flink Table API / SQL, aiming for parity with Spark’s ST_Geog* constructor surface.
Changes:
- Added
flinkTable APIScalarFunctionwrappers for 9 geography constructor/conversion functions. - Registered the new
ST_Geog*functions inflink’sCatalogsoSedonaContext.create(...)can expose them as temporary system functions. - Added end-to-end Table API tests covering the new functions (including geography↔geometry conversions).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| flink/src/main/java/org/apache/sedona/flink/expressions/GeographyConstructors.java | New Flink ScalarFunction wrappers for geography constructors/conversions. |
| flink/src/main/java/org/apache/sedona/flink/Catalog.java | Registers the new geography functions so they’re available via Flink SQL/Table API. |
| flink/src/test/java/org/apache/sedona/flink/GeographyConstructorTest.java | Adds Table API tests to validate function behavior and registration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+21
to
+25
| import org.apache.flink.table.annotation.DataTypeHint; | ||
| import org.apache.flink.table.functions.ScalarFunction; | ||
| import org.apache.sedona.common.S2Geography.Geography; | ||
| import org.apache.sedona.common.geography.Constructors; | ||
| import org.apache.sedona.flink.GeographyTypeSerializer; |
Comment on lines
+126
to
+132
| Geometry point = new WKTReader().read("POINT (3 4)"); | ||
| byte[] wkb = new WKBWriter().write(point); | ||
| Table src = sourceTable("wkb", wkb, PrimitiveArrayTypeInfo.BYTE_PRIMITIVE_ARRAY_TYPE_INFO); | ||
| Geography result = | ||
| evalGeog(src, GeographyConstructors.ST_GeogFromEWKB.class.getSimpleName(), "wkb"); | ||
| assertEquals(Constructors.geogFromWKB(wkb).toEWKT(), result.toEWKT()); | ||
| } |
jiayuasu
force-pushed
the
flink-geography-constructors
branch
from
a39fb43 to
fe95e07
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Did you read the Contributor Guide?
Is this PR related to a ticket?
[GH-XXX] my subject. Closes SedonaFlink: add geography constructor functions (ST_Geog*) #3061.What changes were proposed in this PR?
This is the second step toward Geography type support in SedonaFlink (#3054), building on the
GeographyTypeSerializeradded in #3058.It exposes the geography constructor/conversion functions to the Flink Table API / SQL, reaching parity with the 9 functions Spark already provides:
ST_GeogFromWKT(wkt, [srid])ST_GeogFromText(wkt, [srid])ST_GeogFromEWKT(ewkt)ST_GeogCollFromText(wkt, [srid])ST_GeogFromWKB(wkb, [srid])ST_GeogFromEWKB(wkb)ST_GeogFromGeoHash(geohash, [precision])ST_GeogToGeometry(geography) → geometryST_GeomToGeography(geometry) → geographyEach is a
ScalarFunctionwrapper in a newexpressions/GeographyConstructors.java(mirroring the dedicated geography package on the Spark side), delegating toorg.apache.sedona.common.geography.Constructorswith@DataTypeHint(value = "RAW", rawSerializer = GeographyTypeSerializer.class, bridgedTo = Geography.class). Optional-argument defaults match Spark (srid0, geohash precisionnull). All 9 are registered inCatalog.java.How was this patch tested?
Added
GeographyConstructorTest(10 tests), exercising every function end-to-end through the Flink Table API — including geography↔geometry conversion round-trips. BecauseSedonaContext.createregistersCatalog.getFuncs()as temporary system functions, the tests also verify the Catalog registration.mvn -pl flink test -Dtest=GeographyConstructorTestpasses (10/10), andModuleTestremains green.Did this PR include necessary documentation updates?