beliefer commented on code in PR #43612:
URL: https://github.com/apache/spark/pull/43612#discussion_r1378672003
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala:
##########
@@ -247,6 +251,67 @@ object Literal {
s"Literal must have a corresponding value to ${dataType.catalogString},
" +
s"but class ${Utils.getSimpleName(value.getClass)} found.")
}
+
+ /**
+ * Parse and analyze the .sql string of a Literal, construct the Literal
given the data type json.
+ * Example usage:
+ * val serializedValue = lit.sql
+ * val dataTypeJson = lit.dataType.json
+ * val deserializedLit: Literal = Literal.fromSQL(serializedValue,
dataTypeJson)
+ *
+ * @param valueSqlStr the .sql string of the Literal
+ * @param dataTypeJson the json format data type of the Literal
+ * @throws AnalysisException
INVALID_LITERAL_VALUE_SQL_STRING_FOR_DESERIALIZATION
+ */
+ private[sql] def fromSQL(valueSqlStr: String, dataTypeJson: String): Literal
= {
+ def parseAndAnalyze(valueSqlStr: String, dataType: DataType): Expression =
{
Review Comment:
This method is very similar to
https://github.com/apache/spark/blob/e6b4fa835de3f6d0057bf3809ea369d785967bcd/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ResolveDefaultColumnsUtil.scala#L263
Could we reuse the `ResolveDefaultColumns.analyze`?
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/LiteralExpressionSuite.scala:
##########
@@ -477,4 +477,299 @@ class LiteralExpressionSuite extends SparkFunSuite with
ExpressionEvalHelper {
Literal.create(UTF8String.fromString("Spark SQL"),
ObjectType(classOf[UTF8String])),
UTF8String.fromString("Spark SQL"))
}
+
+ private def serializationRoundTripInfo(
+ before: Literal,
+ serializedValue: String,
+ serializedDataType: String,
+ after: Literal): String = {
+ s"""- Initial Literal: $before with type ${before.dataType}
+ |- String after serialization: $serializedValue with type
$serializedDataType
+ |- Deserialized Literal: $after with type ${after.dataType}
+ |""".stripMargin
+ }
+
+ test(".sql and fromSQL round trip: basic Literals") {
+ Seq(
+ // null
+ Literal.create(null, BooleanType),
+ Literal.create(null, ByteType),
+ Literal.create(null, ShortType),
+ Literal.create(null, IntegerType),
+ Literal.create(null, LongType),
+ Literal.create(null, FloatType),
+ Literal.create(null, StringType),
+ Literal.create(null, BinaryType),
+ Literal.create(null, DecimalType.USER_DEFAULT),
+ Literal.create(null, DecimalType(20, 2)),
+ Literal.create(null, DateType),
+ Literal.create(null, TimestampType),
+ Literal.create(null, CalendarIntervalType),
+ Literal.create(null, YearMonthIntervalType()),
+ Literal.create(null, DayTimeIntervalType(1, 2)),
+ Literal.create(null, ArrayType(ByteType, true)),
+ Literal.create(null, MapType(StringType, IntegerType)),
+ Literal.create(null, StructType(Seq.empty)),
+
+ // boolean
+ Literal(true),
+ Literal(false),
+
+ // int, long, short, byte
+ Literal(0),
+ Literal(1.toLong),
+ Literal(0.toShort),
+ Literal(1.toByte),
+ Literal(Int.MinValue),
+ Literal(Int.MinValue.toShort),
+ Literal(Int.MaxValue),
+ Literal(Int.MaxValue.toByte),
+ Literal(Short.MinValue),
+ Literal(Byte.MaxValue),
+ Literal(Long.MinValue),
+ Literal(Long.MaxValue),
+
+ // float
+ Literal(0.0.toFloat),
+ Literal(-0.0.toFloat),
+ Literal(Float.PositiveInfinity),
+ Literal(Float.NegativeInfinity),
+ Literal(Float.MinPositiveValue),
+ Literal(Float.MaxValue),
+ Literal(Float.MinValue),
+ Literal(Float.NaN),
+
+ // double
+ Literal(0.0),
+ Literal(-0.0),
+ Literal(Double.NegativeInfinity),
+ Literal(Double.PositiveInfinity),
+ Literal(Double.MinValue),
+ Literal(Double.MaxValue),
+ Literal(Double.NaN),
+ Literal(Double.MinPositiveValue),
+
+ // Decimal -> without type it's problematic?
+ Literal(Decimal(-0.0001)),
+ Literal(Decimal(0.0)),
+ Literal(Decimal(0.001)),
+ Literal(Decimal(1.1111)),
+ Literal(Decimal(5.toLong, 10, 3)),
+ Literal(BigDecimal((-0.0001).toString)),
+ Literal(new java.math.BigDecimal(0.0.toString)),
+
+ // binary
+ Literal.create(new Array[Byte](0), BinaryType),
+ Literal.create(new Array[Byte](2), BinaryType),
+
+ // string
+ Literal(""),
+ Literal("a"),
+ Literal("\u0000"),
+ Literal("a b"),
+
+ // DayTimeInterval (Duration)
+ Literal(Duration.ofNanos(0)),
+ Literal(Duration.ofSeconds(-1)),
+ Literal(Duration.ofMinutes(62)),
+ Literal.create(Duration.ofMinutes(62), DayTimeIntervalType(2, 3)),
+ Literal(Duration.ofHours(10)),
+ Literal.create(Duration.ofDays(12345600), DayTimeIntervalType(0, 1)),
+
+ // YearMonthInterval (Period)
+ Literal(Period.ofYears(0)),
+ Literal.create(Period.ofYears(1), YearMonthIntervalType(0, 0)),
+ Literal.create(Period.ofYears(1), YearMonthIntervalType(1, 1)),
+ Literal(Period.of(-1, 11, 0)),
+ Literal(Period.ofMonths(Int.MinValue)),
+
+
Review Comment:
```suggestion
```
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/LiteralExpressionSuite.scala:
##########
@@ -477,4 +477,299 @@ class LiteralExpressionSuite extends SparkFunSuite with
ExpressionEvalHelper {
Literal.create(UTF8String.fromString("Spark SQL"),
ObjectType(classOf[UTF8String])),
UTF8String.fromString("Spark SQL"))
}
+
+ private def serializationRoundTripInfo(
+ before: Literal,
+ serializedValue: String,
+ serializedDataType: String,
+ after: Literal): String = {
+ s"""- Initial Literal: $before with type ${before.dataType}
+ |- String after serialization: $serializedValue with type
$serializedDataType
+ |- Deserialized Literal: $after with type ${after.dataType}
+ |""".stripMargin
+ }
+
+ test(".sql and fromSQL round trip: basic Literals") {
+ Seq(
+ // null
+ Literal.create(null, BooleanType),
+ Literal.create(null, ByteType),
+ Literal.create(null, ShortType),
+ Literal.create(null, IntegerType),
+ Literal.create(null, LongType),
+ Literal.create(null, FloatType),
+ Literal.create(null, StringType),
+ Literal.create(null, BinaryType),
+ Literal.create(null, DecimalType.USER_DEFAULT),
+ Literal.create(null, DecimalType(20, 2)),
+ Literal.create(null, DateType),
+ Literal.create(null, TimestampType),
+ Literal.create(null, CalendarIntervalType),
+ Literal.create(null, YearMonthIntervalType()),
+ Literal.create(null, DayTimeIntervalType(1, 2)),
+ Literal.create(null, ArrayType(ByteType, true)),
+ Literal.create(null, MapType(StringType, IntegerType)),
+ Literal.create(null, StructType(Seq.empty)),
+
+ // boolean
+ Literal(true),
+ Literal(false),
+
+ // int, long, short, byte
+ Literal(0),
+ Literal(1.toLong),
+ Literal(0.toShort),
+ Literal(1.toByte),
+ Literal(Int.MinValue),
+ Literal(Int.MinValue.toShort),
+ Literal(Int.MaxValue),
+ Literal(Int.MaxValue.toByte),
+ Literal(Short.MinValue),
+ Literal(Byte.MaxValue),
+ Literal(Long.MinValue),
+ Literal(Long.MaxValue),
+
+ // float
+ Literal(0.0.toFloat),
+ Literal(-0.0.toFloat),
+ Literal(Float.PositiveInfinity),
+ Literal(Float.NegativeInfinity),
+ Literal(Float.MinPositiveValue),
+ Literal(Float.MaxValue),
+ Literal(Float.MinValue),
+ Literal(Float.NaN),
+
+ // double
+ Literal(0.0),
+ Literal(-0.0),
+ Literal(Double.NegativeInfinity),
+ Literal(Double.PositiveInfinity),
+ Literal(Double.MinValue),
+ Literal(Double.MaxValue),
+ Literal(Double.NaN),
+ Literal(Double.MinPositiveValue),
+
+ // Decimal -> without type it's problematic?
+ Literal(Decimal(-0.0001)),
+ Literal(Decimal(0.0)),
+ Literal(Decimal(0.001)),
+ Literal(Decimal(1.1111)),
+ Literal(Decimal(5.toLong, 10, 3)),
+ Literal(BigDecimal((-0.0001).toString)),
+ Literal(new java.math.BigDecimal(0.0.toString)),
+
+ // binary
+ Literal.create(new Array[Byte](0), BinaryType),
+ Literal.create(new Array[Byte](2), BinaryType),
+
+ // string
+ Literal(""),
+ Literal("a"),
+ Literal("\u0000"),
+ Literal("a b"),
+
+ // DayTimeInterval (Duration)
+ Literal(Duration.ofNanos(0)),
+ Literal(Duration.ofSeconds(-1)),
+ Literal(Duration.ofMinutes(62)),
+ Literal.create(Duration.ofMinutes(62), DayTimeIntervalType(2, 3)),
+ Literal(Duration.ofHours(10)),
+ Literal.create(Duration.ofDays(12345600), DayTimeIntervalType(0, 1)),
+
+ // YearMonthInterval (Period)
+ Literal(Period.ofYears(0)),
+ Literal.create(Period.ofYears(1), YearMonthIntervalType(0, 0)),
+ Literal.create(Period.ofYears(1), YearMonthIntervalType(1, 1)),
+ Literal(Period.of(-1, 11, 0)),
+ Literal(Period.ofMonths(Int.MinValue)),
+
+
+
+ // array
+ Literal(Array(1, 2, 3)),
+ Literal.create(Array(1.0, 2.0), ArrayType(DoubleType, false)),
+ Literal.create(Array(1.0, 2.0), ArrayType(DoubleType, true)),
+ Literal.create(Array(1.0, 2.0, null), ArrayType(DoubleType, true)),
+ Literal(Array("a")),
+
+ // array of struct
+ Literal(
+ new GenericArrayData(
+ Array(
+ InternalRow(UTF8String.fromString("a"), 1),
+ InternalRow(UTF8String.fromString("b"), 2)
+ )
+ ),
+ ArrayType(
+ StructType(Seq(StructField("col1", StringType), StructField("col2",
IntegerType))),
+ containsNull = false
+ )
+ ),
+
+ // struct
+ Literal.create((1, 3.0, "abc")),
+ Literal(
+ InternalRow(true, 1.toLong),
+ StructType(
+ Seq(
+ StructField("col1", BooleanType, nullable =
false).withComment("some-comment"),
+ StructField("col2", LongType)
+ )
+ )
+ ),
+
+ // struct contains array
+ Literal(
+ InternalRow(1.0, new GenericArrayData(Array(true, false, null))),
+ StructType(
+ Seq(
+ StructField("col1", DoubleType),
+ StructField("col2", ArrayType(BooleanType, containsNull = true))
+ )
+ )
+ ),
+
+ // map
+ Literal(
+ new ArrayBasedMapData(
+ new GenericArrayData(Array(UTF8String.fromString("a"),
UTF8String.fromString("b"))),
+ new GenericArrayData(Array(1.toShort, 222.toShort))),
+ MapType(StringType, ShortType, valueContainsNull = false)
+ ),
+
+ // map contains array
+ Literal(
+ new ArrayBasedMapData(
+ new GenericArrayData(Array(UTF8String.fromString("a"),
UTF8String.fromString("b"))),
+ new GenericArrayData(Array(
+ new GenericArrayData(Array(1, 2)), new GenericArrayData(Array(3))
+ ))
+ ),
+ MapType(StringType, ArrayType(IntegerType, containsNull = true))
+ )
+
+ // TODO(anchovyu): enable this test when map value comparison problem is
fixed
+ // mixture of complex data type
+ // [map(
+ // alex -> [(12345, home), (67890, work)],
+ // bob -> [(null, home)]
+ // )]
+ // , Literal(
+ // new GenericArrayData(Array(
+ // new ArrayBasedMapData(
+ // new GenericArrayData(Array(
+ // UTF8String.fromString("alex"),
+ // UTF8String.fromString("bob"))
+ // ),
+ // new GenericArrayData(Array(
+ // new GenericArrayData(Array(
+ // InternalRow(12345, UTF8String.fromString("home")),
+ // InternalRow(67890, UTF8String.fromString("work")))
+ // ),
+ // new GenericArrayData(Array(InternalRow(null,
UTF8String.fromString("home"))))
+ // ))
+ // )
+ // )),
+ // ArrayType(
+ // MapType(
+ // StringType,
+ // ArrayType(
+ // StructType(Seq(
+ // StructField("phone", IntegerType).withComment("phone
number"),
+ // StructField("tags", StringType)
+ // ))
+ // )
+ // )
+ // )
+ // )
Review Comment:
Could we retain the line starts with `// TODO` and delete others?
--
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]
