uros-db commented on code in PR #53009:
URL: https://github.com/apache/spark/pull/53009#discussion_r2517953487
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -570,6 +570,13 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val GEOSPATIAL_ENABLED =
+ buildConf("spark.sql.geospatial.enabled")
+ .doc("When true, enables geospatial types (GEOGRAPHY/GEOMETRY) and ST
functions.")
+ .version("4.1.0")
+ .booleanConf
+ .createWithDefaultFunction(() => Utils.isTesting)
Review Comment:
Not a bug, because there's no real user-facing behavioral difference between
the two approaches. It's just a matter of deferred vs eager evaluation for
testing purposes, which happens to be important for some of the code paths that
we need to guard for GeoSpatial. So, the existing code that you attached is
fine for other configs, but for geo we need to use lazy eval in order to avoid
freezing the default when reading the value too quickly in tests.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/st/stExpressions.scala:
##########
@@ -17,12 +17,53 @@
package org.apache.spark.sql.catalyst.expressions.st
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.analysis.TypeCheckResult
import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.expressions.codegen.CodegenFallback
import org.apache.spark.sql.catalyst.expressions.objects._
import org.apache.spark.sql.catalyst.trees._
import org.apache.spark.sql.catalyst.util.{Geography, Geometry, STUtils}
+import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
+/**
+ * ST expressions are behind a feature flag while the geospatial module is
under development.
+ */
+
+private[sql] trait GeospatialAnalysisGuard extends Expression {
+ override def checkInputDataTypes(): TypeCheckResult = {
+ if (!SQLConf.get.geospatialEnabled) {
+ TypeCheckResult.TypeCheckFailure(
+ "Geospatial feature is disabled. Set
\"spark.sql.geospatial.enabled=true\" to enable."
Review Comment:
I think we don't actually need this anymore, removed it. The other guard
should be sufficient.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
