Skip to content

Commit 18a9435

Browse files
davidm-dbcloud-fan
authored andcommitted
[SPARK-54609][SQL] Disable TIME type by default
### What changes were proposed in this pull request? Introducing a new SQL config for TIME type: `spark.sql.timeType.enabled`. The default value is `false` and it is enabled only in tests. ### Why are the changes needed? TIME data type support is not complete, so we need to guard it before it is completed, especially ahead of Spark 4.1 release. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Need to add tests for disabled config. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #53344 from davidm-db/davidm-db/time-config. Lead-authored-by: David Milicevic <[email protected]> Co-authored-by: Wenchen Fan <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
1 parent d80a0f3 commit 18a9435

File tree

9 files changed

+75
-18
lines changed

9 files changed

+75
-18
lines changed

common/utils/src/main/resources/error/error-conditions.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7309,6 +7309,12 @@
73097309
],
73107310
"sqlState" : "0A001"
73117311
},
7312+
"UNSUPPORTED_TIME_TYPE" : {
7313+
"message" : [
7314+
"The data type TIME is not supported."
7315+
],
7316+
"sqlState" : "0A000"
7317+
},
73127318
"UNSUPPORTED_TYPED_LITERAL" : {
73137319
"message" : [
73147320
"Literals of the type <unsupportedType> are not supported. Supported types are <supportedTypes>."

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,11 @@ import java.time.{Duration, Instant, LocalDate, LocalDateTime, LocalTime, Period
2525
import java.util.{Map => JavaMap}
2626
import javax.annotation.Nullable
2727

28-
import scala.language.existentials
29-
3028
import org.apache.spark.SparkIllegalArgumentException
3129
import org.apache.spark.sql.Row
3230
import org.apache.spark.sql.catalyst.expressions._
3331
import org.apache.spark.sql.catalyst.util._
32+
import org.apache.spark.sql.errors.QueryCompilationErrors
3433
import org.apache.spark.sql.internal.SQLConf
3534
import org.apache.spark.sql.types._
3635
import org.apache.spark.sql.types.DayTimeIntervalType._
@@ -79,6 +78,8 @@ object CatalystTypeConverters {
7978
new GeometryConverter(g)
8079
case DateType if SQLConf.get.datetimeJava8ApiEnabled => LocalDateConverter
8180
case DateType => DateConverter
81+
case _: TimeType if !SQLConf.get.isTimeTypeEnabled =>
82+
QueryCompilationErrors.unsupportedTimeTypeError()
8283
case _: TimeType => TimeConverter
8384
case TimestampType if SQLConf.get.datetimeJava8ApiEnabled => InstantConverter
8485
case TimestampType => TimestampConverter

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import org.apache.spark.sql.catalyst.util._
3535
import org.apache.spark.sql.catalyst.util.DateTimeConstants._
3636
import org.apache.spark.sql.catalyst.util.DateTimeUtils._
3737
import org.apache.spark.sql.catalyst.util.IntervalUtils.{dayTimeIntervalToByte, dayTimeIntervalToDecimal, dayTimeIntervalToInt, dayTimeIntervalToLong, dayTimeIntervalToShort, yearMonthIntervalToByte, yearMonthIntervalToInt, yearMonthIntervalToShort}
38-
import org.apache.spark.sql.errors.{QueryErrorsBase, QueryExecutionErrors}
38+
import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryErrorsBase, QueryExecutionErrors}
3939
import org.apache.spark.sql.internal.SQLConf
4040
import org.apache.spark.sql.types._
4141
import org.apache.spark.unsafe.types.{GeographyVal, UTF8String, VariantVal}
@@ -617,6 +617,12 @@ case class Cast(
617617
}
618618

619619
override def checkInputDataTypes(): TypeCheckResult = {
620+
dataType match {
621+
// If the cast is to a TIME type, first check if TIME type is enabled.
622+
case _: TimeType if !SQLConf.get.isTimeTypeEnabled =>
623+
throw QueryCompilationErrors.unsupportedTimeTypeError()
624+
case _ =>
625+
}
620626
val canCast = evalMode match {
621627
case EvalMode.LEGACY => Cast.canCast(child.dataType, dataType)
622628
case EvalMode.ANSI => Cast.canAnsiCast(child.dataType, dataType)

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,22 @@ import org.apache.spark.sql.catalyst.util.DateTimeUtils
3434
import org.apache.spark.sql.catalyst.util.TimeFormatter
3535
import org.apache.spark.sql.catalyst.util.TypeUtils.ordinalNumber
3636
import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryExecutionErrors}
37+
import org.apache.spark.sql.internal.SQLConf
3738
import org.apache.spark.sql.internal.types.StringTypeWithCollation
3839
import org.apache.spark.sql.types.{AbstractDataType, AnyTimeType, ByteType, DataType, DayTimeIntervalType, Decimal, DecimalType, DoubleType, FloatType, IntegerType, IntegralType, LongType, NumericType, ObjectType, TimeType}
3940
import org.apache.spark.sql.types.DayTimeIntervalType.{HOUR, SECOND}
4041
import org.apache.spark.unsafe.types.UTF8String
4142

43+
trait TimeExpression extends Expression {
44+
override def checkInputDataTypes(): TypeCheckResult = {
45+
if (SQLConf.get.isTimeTypeEnabled) {
46+
super.checkInputDataTypes()
47+
} else {
48+
throw QueryCompilationErrors.unsupportedTimeTypeError()
49+
}
50+
}
51+
}
52+
4253
/**
4354
* Parses a column to a time based on the given format.
4455
*/
@@ -66,7 +77,7 @@ import org.apache.spark.unsafe.types.UTF8String
6677
since = "4.1.0")
6778
// scalastyle:on line.size.limit
6879
case class ToTime(str: Expression, format: Option[Expression])
69-
extends RuntimeReplaceable with ExpectsInputTypes {
80+
extends RuntimeReplaceable with ExpectsInputTypes with TimeExpression {
7081

7182
def this(str: Expression, format: Expression) = this(str, Option(format))
7283
def this(str: Expression) = this(str, None)
@@ -202,7 +213,7 @@ object TryToTimeExpressionBuilder extends ExpressionBuilder {
202213
// scalastyle:on line.size.limit
203214
case class MinutesOfTime(child: Expression)
204215
extends RuntimeReplaceable
205-
with ExpectsInputTypes {
216+
with ExpectsInputTypes with TimeExpression {
206217

207218
override def replacement: Expression = StaticInvoke(
208219
classOf[DateTimeUtils.type],
@@ -261,7 +272,7 @@ object MinuteExpressionBuilder extends ExpressionBuilder {
261272

262273
case class HoursOfTime(child: Expression)
263274
extends RuntimeReplaceable
264-
with ExpectsInputTypes {
275+
with ExpectsInputTypes with TimeExpression {
265276

266277
override def replacement: Expression = StaticInvoke(
267278
classOf[DateTimeUtils.type],
@@ -318,7 +329,7 @@ object HourExpressionBuilder extends ExpressionBuilder {
318329

319330
case class SecondsOfTimeWithFraction(child: Expression)
320331
extends RuntimeReplaceable
321-
with ExpectsInputTypes {
332+
with ExpectsInputTypes with TimeExpression {
322333
override def replacement: Expression = {
323334
val precision = child.dataType match {
324335
case TimeType(p) => p
@@ -344,7 +355,7 @@ case class SecondsOfTimeWithFraction(child: Expression)
344355

345356
case class SecondsOfTime(child: Expression)
346357
extends RuntimeReplaceable
347-
with ExpectsInputTypes {
358+
with ExpectsInputTypes with TimeExpression {
348359

349360
override def replacement: Expression = StaticInvoke(
350361
classOf[DateTimeUtils.type],
@@ -435,7 +446,8 @@ object SecondExpressionBuilder extends ExpressionBuilder {
435446
case class CurrentTime(
436447
child: Expression = Literal(TimeType.MICROS_PRECISION),
437448
timeZoneId: Option[String] = None) extends UnaryExpression
438-
with TimeZoneAwareExpression with ImplicitCastInputTypes with CodegenFallback {
449+
with TimeZoneAwareExpression with ImplicitCastInputTypes with CodegenFallback
450+
with TimeExpression {
439451

440452
def this() = {
441453
this(Literal(TimeType.MICROS_PRECISION), None)
@@ -547,7 +559,7 @@ case class MakeTime(
547559
secsAndMicros: Expression)
548560
extends RuntimeReplaceable
549561
with ImplicitCastInputTypes
550-
with ExpectsInputTypes {
562+
with ExpectsInputTypes with TimeExpression {
551563

552564
// Accept `sec` as DecimalType to avoid loosing precision of microseconds while converting
553565
// it to the fractional part of `sec`. If `sec` is an IntegerType, it can be cast into decimal
@@ -572,7 +584,8 @@ case class MakeTime(
572584
* Adds day-time interval to time.
573585
*/
574586
case class TimeAddInterval(time: Expression, interval: Expression)
575-
extends BinaryExpression with RuntimeReplaceable with ExpectsInputTypes {
587+
extends BinaryExpression with RuntimeReplaceable with ExpectsInputTypes
588+
with TimeExpression {
576589
override def nullIntolerant: Boolean = true
577590

578591
override def left: Expression = time
@@ -613,7 +626,8 @@ case class TimeAddInterval(time: Expression, interval: Expression)
613626
* Returns a day-time interval between time values.
614627
*/
615628
case class SubtractTimes(left: Expression, right: Expression)
616-
extends BinaryExpression with RuntimeReplaceable with ExpectsInputTypes {
629+
extends BinaryExpression with RuntimeReplaceable with ExpectsInputTypes
630+
with TimeExpression {
617631
override def nullIntolerant: Boolean = true
618632
override def inputTypes: Seq[AbstractDataType] = Seq(AnyTimeType, AnyTimeType)
619633

@@ -670,7 +684,8 @@ case class TimeDiff(
670684
end: Expression)
671685
extends TernaryExpression
672686
with RuntimeReplaceable
673-
with ImplicitCastInputTypes {
687+
with ImplicitCastInputTypes
688+
with TimeExpression {
674689

675690
override def first: Expression = unit
676691
override def second: Expression = start
@@ -725,7 +740,8 @@ case class TimeDiff(
725740
since = "4.1.0")
726741
// scalastyle:on line.size.limit
727742
case class TimeTrunc(unit: Expression, time: Expression)
728-
extends BinaryExpression with RuntimeReplaceable with ImplicitCastInputTypes {
743+
extends BinaryExpression with RuntimeReplaceable with ImplicitCastInputTypes
744+
with TimeExpression {
729745

730746
override def left: Expression = unit
731747
override def right: Expression = time
@@ -753,7 +769,8 @@ case class TimeTrunc(unit: Expression, time: Expression)
753769
}
754770

755771
abstract class IntegralToTimeBase
756-
extends UnaryExpression with ExpectsInputTypes with CodegenFallback {
772+
extends UnaryExpression with ExpectsInputTypes with CodegenFallback
773+
with TimeExpression {
757774
protected def upScaleFactor: Long
758775

759776
override def inputTypes: Seq[AbstractDataType] = Seq(IntegralType)
@@ -772,7 +789,8 @@ abstract class IntegralToTimeBase
772789
}
773790
}
774791

775-
abstract class TimeToLongBase extends UnaryExpression with ExpectsInputTypes {
792+
abstract class TimeToLongBase extends UnaryExpression with ExpectsInputTypes
793+
with TimeExpression {
776794
protected def scaleFactor: Long
777795

778796
override def inputTypes: Seq[AbstractDataType] = Seq(AnyTimeType)
@@ -819,7 +837,8 @@ abstract class TimeToLongBase extends UnaryExpression with ExpectsInputTypes {
819837
group = "datetime_funcs")
820838
// scalastyle:on line.size.limit
821839
case class TimeFromSeconds(child: Expression)
822-
extends UnaryExpression with ExpectsInputTypes with CodegenFallback {
840+
extends UnaryExpression with ExpectsInputTypes with CodegenFallback
841+
with TimeExpression {
823842
override def inputTypes: Seq[AbstractDataType] = Seq(NumericType)
824843
override def dataType: DataType = TimeType(TimeType.MICROS_PRECISION)
825844
override def nullable: Boolean = true

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4492,4 +4492,10 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase with Compilat
44924492
"colType" -> "metadata",
44934493
"errors" -> errors.mkString("- ", "\n- ", "")))
44944494
}
4495+
4496+
def unsupportedTimeTypeError(): Throwable = {
4497+
new AnalysisException(
4498+
errorClass = "UNSUPPORTED_TIME_TYPE",
4499+
messageParameters = Map.empty)
4500+
}
44954501
}

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6731,6 +6731,13 @@ object SQLConf {
67316731
.booleanConf
67326732
.createWithDefault(false)
67336733

6734+
val TIME_TYPE_ENABLED =
6735+
buildConf("spark.sql.timeType.enabled")
6736+
.doc("When true, the TIME data type is supported.")
6737+
.version("4.1.0")
6738+
.booleanConf
6739+
.createWithDefault(Utils.isTesting)
6740+
67346741
/**
67356742
* Holds information about keys that have been deprecated.
67366743
*
@@ -7938,6 +7945,8 @@ class SQLConf extends Serializable with Logging with SqlApiConf {
79387945
def coerceMergeNestedTypes: Boolean =
79397946
getConf(SQLConf.MERGE_INTO_NESTED_TYPE_COERCION_ENABLED)
79407947

7948+
def isTimeTypeEnabled: Boolean = getConf(SQLConf.TIME_TYPE_ENABLED)
7949+
79417950
/** ********************** SQLConf functionality methods ************ */
79427951

79437952
/** Set Spark SQL configuration properties. */

sql/connect/server/src/main/scala/org/apache/spark/sql/connect/execution/SparkConnectPlanExecution.scala

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,11 @@ import org.apache.spark.sql.connect.config.Connect.{CONNECT_GRPC_ARROW_MAX_BATCH
3737
import org.apache.spark.sql.connect.planner.{InvalidInputErrors, SparkConnectPlanner}
3838
import org.apache.spark.sql.connect.service.ExecuteHolder
3939
import org.apache.spark.sql.connect.utils.{MetricGenerator, PipelineAnalysisContextUtils}
40+
import org.apache.spark.sql.errors.QueryCompilationErrors
4041
import org.apache.spark.sql.execution.{DoNotCleanup, LocalTableScanExec, QueryExecution, RemoveShuffleFiles, SkipMigration, SQLExecution}
4142
import org.apache.spark.sql.execution.arrow.ArrowConverters
4243
import org.apache.spark.sql.internal.SQLConf
43-
import org.apache.spark.sql.types.{DataType, StructType}
44+
import org.apache.spark.sql.types.{DataType, StructType, TimeType}
4445
import org.apache.spark.util.ThreadUtils
4546

4647
/**
@@ -143,6 +144,10 @@ private[execution] class SparkConnectPlanExecution(executeHolder: ExecuteHolder)
143144
errorClass = "UNSUPPORTED_FEATURE.GEOSPATIAL_DISABLED",
144145
messageParameters = scala.collection.immutable.Map.empty)
145146
}
147+
val timeTypeEnabled = spark.sessionState.conf.isTimeTypeEnabled
148+
if (!timeTypeEnabled && schema.existsRecursively(_.isInstanceOf[TimeType])) {
149+
throw QueryCompilationErrors.unsupportedTimeTypeError()
150+
}
146151
val maxRecordsPerBatch = spark.sessionState.conf.arrowMaxRecordsPerBatch
147152
val timeZoneId = spark.sessionState.conf.sessionLocalTimeZone
148153
val largeVarTypes = spark.sessionState.conf.arrowUseLargeVarTypes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceUtils.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ object DataSourceUtils extends PredicateHelper {
9393
* in a driver side.
9494
*/
9595
def verifySchema(format: FileFormat, schema: StructType, readOnly: Boolean = false): Unit = {
96+
if (!SQLConf.get.isTimeTypeEnabled && schema.existsRecursively(_.isInstanceOf[TimeType])) {
97+
throw QueryCompilationErrors.unsupportedTimeTypeError()
98+
}
9699
schema.foreach { field =>
97100
val supported = if (readOnly) {
98101
format.supportReadDataType(field.dataType)

sql/gen-sql-functions-docs.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,8 @@ def generate_functions_examples_html(jvm, jspark, html_output_dir):
240240
</pre></div>
241241
242242
"""
243+
print("Enabling TIME data type")
244+
jspark.sql("SET spark.sql.timeType.enabled = true")
243245
print("Running SQL examples to generate formatted output.")
244246
for key, infos in _list_grouped_function_infos(jvm):
245247
examples = _make_pretty_examples(jspark, infos)

0 commit comments

Comments
 (0)