You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "zhangfengcdt (via GitHub)" <gi...@apache.org> on 2025/10/22 02:37:14 UTC

[PR] [GH-2419] Register Sedona functions as built-in functions to support permanent VIEW creation [sedona]

zhangfengcdt opened a new pull request, #2420:
URL: https://github.com/apache/sedona/pull/2420

   ## Did you read the Contributor Guide?
   
   - Yes, I have read the [Contributor Rules](https://sedona.apache.org/latest/community/rule/) and [Contributor Development Guide](https://sedona.apache.org/latest/community/develop/)
   
   ## Is this PR related to a ticket?
   
   - Yes, and the PR name follows the format `[GH-XXX] my subject`. Closes #<issue_number>
   
   ## What changes were proposed in this PR?
   Sedona functions are registered as temporary functions in the session `FunctionRegistry`, which prevents creating permanent VIEWs that reference them:
   
     ```sql
     CREATE VIEW my_view AS
     SELECT ST_AsText(geom) FROM buildings;
   ```
   
     -- Error: Cannot create view that references temporary function: ST_AsText
   
   Spark's CreateViewCommand validates that permanent VIEWs cannot reference temporary functions. Since Sedona functions are only registered in the session registry (not the built-in registry), they are treated as temporary.
   
   This PR registers Sedona functions into Spark's built-in function registry (FunctionRegistry.builtin) in addition to the session registry. This makes Spark treat Sedona functions as built-in functions (similar to COUNT, MAX, SUM) rather than temporary functions, bypassing the CREATE VIEW validation.
   
   ## How was this patch tested?
    The new test validates:
     - Permanent VIEW creation with Sedona spatial functions
     - VIEW queries return correct results
     - Proper assertions instead of console output
   
   ## Did this PR include necessary documentation updates?
   
   - No, this PR does not affect any public API so no need to change the documentation.
   


-- 
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] [GH-2419] Register Sedona functions as built-in functions to support permanent VIEW creation [sedona]

Posted by "zhangfengcdt (via GitHub)" <gi...@apache.org>.
zhangfengcdt commented on code in PR #2420:
URL: https://github.com/apache/sedona/pull/2420#discussion_r2452258398


##########
spark/common/src/main/scala/org/apache/sedona/sql/UDF/AbstractCatalog.scala:
##########
@@ -86,6 +87,10 @@ abstract class AbstractCatalog {
         functionIdentifier,
         expressionInfo,
         functionBuilder)
+      FunctionRegistry.builtin.registerFunction(

Review Comment:
   It looks like we need to register to both the `sessionState.functionRegistry` and the buildin simple function registry. They server different purpose, and if we remove the first one, functions cannot be used any more.
   
   I have added the three aggregate functions to buildin registry as well and also new test on creating views on spatial aggregation queries.
   



-- 
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] [GH-2419] Register Sedona functions as built-in functions to support permanent VIEW creation [sedona]

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


-- 
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] [GH-2419] Register Sedona functions as built-in functions to support permanent VIEW creation [sedona]

Posted by "haizhou-zhao (via GitHub)" <gi...@apache.org>.
haizhou-zhao commented on code in PR #2420:
URL: https://github.com/apache/sedona/pull/2420#discussion_r2452872125


##########
spark/common/src/main/scala/org/apache/sedona/sql/UDF/AbstractCatalog.scala:
##########
@@ -86,9 +87,20 @@ abstract class AbstractCatalog {
         functionIdentifier,
         expressionInfo,
         functionBuilder)
+      FunctionRegistry.builtin.registerFunction(
+        functionIdentifier,
+        expressionInfo,
+        functionBuilder)
+    }
+    aggregateExpressions.foreach { f =>
+      sparkSession.udf.register(f.getClass.getSimpleName, functions.udaf(f))
+      FunctionRegistry.builtin.registerFunction(

Review Comment:
   For folks in the industry who're using HiveMetastore, IcebergRestCatalog or other table metastore options: might be a good idea to leave an option open for them to register geospatial related udf with various metastore options if they prefer to do things that way (i.e. one draw back I could think of with having too many buildin func is it complicates the resolving path for every command, whether or not they contain geospatial udf). Don't have to do this in this PR, but might be great to have a tracking issue or TODO in some form.



-- 
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] [GH-2419] Register Sedona functions as built-in functions to support permanent VIEW creation [sedona]

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

   Thanks. Merging.


-- 
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] [GH-2419] Register Sedona functions as built-in functions to support permanent VIEW creation [sedona]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2420:
URL: https://github.com/apache/sedona/pull/2420#discussion_r2450309178


##########
spark/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala:
##########
@@ -4130,4 +4132,70 @@ class functionTestScala
       squareWithTwoHolesSimplified.first().getAs[org.locationtech.jts.geom.Geometry](0)
     assert(simplifiedMedialAxis != null, "Simplified medial axis should not be null")
   }
+
+  it("Test that CREATE VIEW fails with multiple temporary Sedona functions") {
+    val timestamp = System.currentTimeMillis()
+    val tmpDir: String =
+      Files.createTempDirectory("sedona_geoparquet_test_").toFile.getAbsolutePath
+
+    val buildings = sparkSession.sql("""
+      SELECT
+        ST_GeomFromWKT('POLYGON((0 0, 1 0, 1 1, 0 1, 0 0))') as geom,
+        'Building 1' as PROP_ADDR
+      UNION ALL
+      SELECT
+        ST_GeomFromWKT('POLYGON((2 2, 3 2, 3 3, 2 3, 2 2))') as geom,
+        'Building 2' as PROP_ADDR
+    """)
+    buildings.write
+      .mode("overwrite")
+      .option("path", s"$tmpDir/sedona_test_${timestamp}_buildings")
+      .saveAsTable("nyc_buildings_geom_test")
+
+    val zones = sparkSession.sql("""
+      SELECT
+        ST_GeomFromWKT('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))') as zone_geom,
+        100.0 as elevation
+    """)
+    zones.write
+      .mode("overwrite")
+      .option("path", s"$tmpDir/sedona_test_${timestamp}_zones")
+      .saveAsTable("elevation_zones_test")
+
+    // Attempt to create a permanent VIEW with multiple Sedona functions
+    sparkSession.sql("""
+      CREATE VIEW nyc_buildings_with_functions AS
+      SELECT * FROM (
+        SELECT
+          nyc_buildings_geom_test.PROP_ADDR AS name,
+          nyc_buildings_geom_test.geom AS building_geom,
+          avg(elevation_zones_test.elevation) AS elevation
+        FROM
+          nyc_buildings_geom_test
+        JOIN
+          elevation_zones_test
+        ON
+          st_intersects(elevation_zones_test.zone_geom, elevation_zones_test.zone_geom)

Review Comment:
   The st_intersects function compares elevation_zones_test.zone_geom with itself, which will always return true. This should likely be st_intersects(elevation_zones_test.zone_geom, nyc_buildings_geom_test.geom) to test the actual spatial relationship between buildings and zones.
   ```suggestion
             st_intersects(nyc_buildings_geom_test.geom, elevation_zones_test.zone_geom)
   ```



-- 
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] [GH-2419] Register Sedona functions as built-in functions to support permanent VIEW creation [sedona]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #2420:
URL: https://github.com/apache/sedona/pull/2420#discussion_r2450358974


##########
spark/common/src/main/scala/org/apache/sedona/sql/UDF/AbstractCatalog.scala:
##########
@@ -86,6 +87,10 @@ abstract class AbstractCatalog {
         functionIdentifier,
         expressionInfo,
         functionBuilder)
+      FunctionRegistry.builtin.registerFunction(

Review Comment:
   Can you confirm if this gets each function registered twice? (once for temp function, one for built-in)?
   
   How about `aggregateExpressions?`



-- 
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] [GH-2419] Register Sedona functions as built-in functions to support permanent VIEW creation [sedona]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #2420:
URL: https://github.com/apache/sedona/pull/2420#discussion_r2452929647


##########
spark/common/src/main/scala/org/apache/sedona/sql/UDF/AbstractCatalog.scala:
##########
@@ -86,9 +87,20 @@ abstract class AbstractCatalog {
         functionIdentifier,
         expressionInfo,
         functionBuilder)
+      FunctionRegistry.builtin.registerFunction(
+        functionIdentifier,
+        expressionInfo,
+        functionBuilder)
+    }
+    aggregateExpressions.foreach { f =>
+      sparkSession.udf.register(f.getClass.getSimpleName, functions.udaf(f))
+      FunctionRegistry.builtin.registerFunction(

Review Comment:
   @haizhou-zhao pleas feel free to create a github issue on Sedona and we can then track it



-- 
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