Skip to content

[SPARK-54253][Geo][SQL] Add a guarding config for geospatial support - #53009

Closed
uros-db wants to merge 22 commits into
apache:masterfrom
uros-db:geo-config
Closed

[SPARK-54253][Geo][SQL] Add a guarding config for geospatial support#53009
uros-db wants to merge 22 commits into
apache:masterfrom
uros-db:geo-config

Conversation

@uros-db

@uros-dburos-db commented Nov 12, 2025

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Introduce a new SQL config for controlling the geospatial feature:

spark.sql.geospatial.enabled

The default value is false, and enabled only in testing.

Why are the changes needed?

Guard the geospatial feature until it's fully finished.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Added appropriate unit tests to confirm that the config is effective:

  • STExpressionsSuite

Was this patch authored or co-authored using generative AI tooling?

No.

@uros-dburos-db changed the title [SPARK-54253] Add a guarding config for geospatial support[SPARK-54253][Geo][SQL] Add a guarding config for geospatial supportNov 12, 2025
dongjoon-hyun
dongjoon-hyun reviewed Nov 12, 2025
View reviewed changes

@dongjoon-hyundongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! Thank you so much for proposing this new configuration, @uros-db .

@uros-db
uros-db marked this pull request as draft November 12, 2025 11:34
uros-db
uros-db commented Nov 12, 2025
View reviewed changes

@uros-dburos-db left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I'm converting this PR to draft while I figure out how to enforce the config in testing for Connect (e.g. ArrowEncoderSuite) and clear those failures. Not sure how to do this at the moment, because static SQL config values can't be changed, and modifying testConfigs in SparkConnectServerUtils doesn't help either because isTesting stays false and config value is not propagated between server & client.

@dongjoon-hyun

dongjoon-hyun commented Nov 13, 2025

Copy link
Copy Markdown
Member

Gentle ping, @uros-db . If there is no way, please turn it false always and enable it inside test cases.

dillitz
dillitz reviewed Nov 14, 2025
View reviewed changes
Comment threadsql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/DataTypeAstBuilder.scala Outdated
milastdbx
milastdbx reviewed Nov 14, 2025
View reviewed changes
Comment threadsql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/AgnosticEncoder.scala Outdated
Comment threadsql/api/src/main/scala/org/apache/spark/sql/types/GeographyType.scala Outdated
Comment threadsql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/st/stExpressions.scala Outdated
Comment thread.../client/jvm/src/test/scala/org/apache/spark/sql/connect/client/arrow/ArrowEncoderSuite.scala Outdated
holdenk
holdenk reviewed Nov 14, 2025
View reviewed changes
Comment threadsql/api/src/main/scala/org/apache/spark/sql/types/GeometryType.scala Outdated
holdenk
holdenk reviewed Nov 14, 2025
View reviewed changes
Comment threadsql/api/src/main/scala/org/apache/spark/sql/types/GeometryType.scala Outdated
@dongjoon-hyun

dongjoon-hyun commented Nov 17, 2025

Copy link
Copy Markdown
Member

Please let me know when you are ready, @uros-db .

cloud-fan
cloud-fan reviewed Nov 18, 2025
View reviewed changes
Comment thread...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/st/STExpressionUtils.scala
*/
def geospatialTypeWithSrid(sourceType: DataType, srid: Expression): DataType = {
sourceType match {
case _ if !SQLConf.get.geospatialEnabled =>

@cloud-fancloud-fanNov 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it a narrow waist for geo functionalities?

@uros-dburos-dbNov 18, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, this is very narrow wrt. geo. It's exclusively used in ST expressions (currently only 1 expression, but will be re-used in a few more) to determine the resulting dataType.

cloud-fan
cloud-fan reviewed Nov 18, 2025
View reviewed changes
Comment threadsql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/types/PhysicalDataType.scala Outdated
cloud-fan
cloud-fan reviewed Nov 18, 2025
View reviewed changes
Comment threadsql/catalyst/src/test/scala/org/apache/spark/sql/types/DataTypeSuite.scala Outdated
cloud-fan
cloud-fan reviewed Nov 18, 2025
View reviewed changes
Comment threadsql/core/src/test/scala/org/apache/spark/sql/RowSuite.scala Outdated
cloud-fan
cloud-fan reviewed Nov 18, 2025
View reviewed changes
Comment threadsql/core/src/test/scala/org/apache/spark/sql/STExpressionsSuite.scala
cloud-fan
cloud-fan reviewed Nov 19, 2025
View reviewed changes
Comment threadsql/core/src/test/scala/org/apache/spark/sql/STExpressionsSuite.scala Outdated
@uros-db
uros-db marked this pull request as ready for review November 19, 2025 10:02
@uros-db

uros-db commented Nov 19, 2025

Copy link
Copy Markdown
ContributorAuthor

Hi folks all checks are good here, and I marked the PR ready as for review. @cloud-fan@dongjoon-hyun@holdenk

@cloud-fan

cloud-fan commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

thanks, merging to master/4.1!

cloud-fan pushed a commit that referenced this pull request Nov 19, 2025
@uros-db@cloud-fan
[SPARK-54253][GEO][SQL] Add a guarding config for geospatial support
2daaa09
### What changes were proposed in this pull request?
Introduce a new SQL config for controlling the geospatial feature:
```
spark.sql.geospatial.enabled
```
The default value is `false`, and enabled only in testing.
### Why are the changes needed?
Guard the geospatial feature until it's fully finished.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added appropriate unit tests to confirm that the config is effective:
- `STExpressionsSuite`
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes#53009 from uros-db/geo-config.
Authored-by: Uros Bojanic <uros.bojanic@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit d299684)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@dongjoon-hyun

dongjoon-hyun commented Nov 19, 2025

Copy link
Copy Markdown
Member

Thank you so much, @uros-db , @cloud-fan and all.

huangxiaopingRD pushed a commit to huangxiaopingRD/spark that referenced this pull request Nov 25, 2025
@uros-db@huangxiaopingRD
[SPARK-54253][GEO][SQL] Add a guarding config for geospatial support
8665ed3
### What changes were proposed in this pull request?
Introduce a new SQL config for controlling the geospatial feature:
```
spark.sql.geospatial.enabled
```
The default value is `false`, and enabled only in testing.
### Why are the changes needed?
Guard the geospatial feature until it's fully finished.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added appropriate unit tests to confirm that the config is effective:
- `STExpressionsSuite`
### Was this patch authored or co-authored using generative AI tooling?
No.
Closesapache#53009 from uros-db/geo-config.
Authored-by: Uros Bojanic <uros.bojanic@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@uros-dburos-db mentioned this pull request Dec 3, 2025
Closed
cloud-fan pushed a commit that referenced this pull request Dec 4, 2025
@uros-db@cloud-fan
[SPARK-54253][GEO][CONNECT][FOLLOWUP] Enforce geospatial support conf…
38d6010
…ig in Spark Connect
### What changes were proposed in this pull request?
A new SQL config (`spark.sql.geospatial.enabled`) was introduced as part of #53009.
However, the original PR didn't fully gate geospatial functionality, so this PR also forbids geo dataframes in Spark Connect.
### Why are the changes needed?
Guard the geospatial feature until it's fully finished.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added appropriate unit tests to confirm that the config is effective:
- `GeographyConnectDataFrameSuite`
- `GeometryConnectDataFrameSuite`
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes#53259 from uros-db/geo-config_connect_tests.
Authored-by: Uros Bojanic <uros.bojanic@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
cloud-fan pushed a commit that referenced this pull request Dec 4, 2025
@uros-db@cloud-fan
[SPARK-54253][GEO][CONNECT][FOLLOWUP] Enforce geospatial support conf…
9f3df27
…ig in Spark Connect
### What changes were proposed in this pull request?
A new SQL config (`spark.sql.geospatial.enabled`) was introduced as part of #53009.
However, the original PR didn't fully gate geospatial functionality, so this PR also forbids geo dataframes in Spark Connect.
### Why are the changes needed?
Guard the geospatial feature until it's fully finished.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added appropriate unit tests to confirm that the config is effective:
- `GeographyConnectDataFrameSuite`
- `GeometryConnectDataFrameSuite`
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes#53259 from uros-db/geo-config_connect_tests.
Authored-by: Uros Bojanic <uros.bojanic@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 38d6010)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cloud-fancloud-fancloud-fan approved these changes
@dongjoon-hyundongjoon-hyunAwaiting requested review from dongjoon-hyun
@holdenkholdenkAwaiting requested review from holdenk
+2 more reviewers
@dillitzdillitzdillitz left review comments
@milastdbxmilastdbxmilastdbx left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

SQL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@uros-db@dongjoon-hyun@cloud-fan@holdenk@dillitz@milastdbx