srielau commented on code in PR #58581:
URL: https://github.com/apache/spark/pull/58581#discussion_r4076336707
##########
sql/api/src/main/scala/org/apache/spark/sql/types/StructField.scala:
##########
@@ -96,18 +98,44 @@ case class StructField(
}
private def metadataJson: JValue = {
- val metadataJsonValue = metadata.jsonValue
- metadataJsonValue match {
- case JObject(fields) if collationMetadata.nonEmpty =>
- val collationFields = collationMetadata.map(kv => kv._1 ->
JString(kv._2)).toList
- JObject(fields :+ (DataType.COLLATIONS_METADATA_KEY ->
JObject(collationFields)))
-
- case _ => metadataJsonValue
+ if (metadata.contains(DataType.CHAR_VARCHAR_COLLATIONS_METADATA_KEY)) {
+ throw new SparkIllegalArgumentException(
+ errorClass = "INVALID_JSON_DATA_TYPE_FOR_COLLATIONS",
+ messageParameters = Map("jsonType" ->
DataType.CHAR_VARCHAR_COLLATIONS_METADATA_KEY))
+ }
+ metadata.jsonValue match {
+ case JObject(fields) =>
+ val withString =
+ if (stringCollationMetadata.nonEmpty) {
+ val collationFields =
+ stringCollationMetadata.map(kv => kv._1 -> JString(kv._2)).toList
+ fields :+ (DataType.COLLATIONS_METADATA_KEY ->
JObject(collationFields))
+ } else {
+ fields
+ }
+ val withBoth =
+ if (charVarcharCollationMetadata.nonEmpty) {
+ val collationFields =
+ charVarcharCollationMetadata.map(kv => kv._1 ->
JString(kv._2)).toList
+ withString :+
+ (DataType.CHAR_VARCHAR_COLLATIONS_METADATA_KEY ->
JObject(collationFields))
+ } else {
+ withString
+ }
+ JObject(withBoth)
+ case other => other
}
}
- /** Map of field path to collation name. */
- private lazy val collationMetadata: Map[String, String] = {
+ /** Map of field path to STRING collation name. */
+ private lazy val stringCollationMetadata: Map[String, String] =
+ collectCollationMetadata(isCollatedPlainString)
+
+ /** Map of field path to CHAR/VARCHAR collation name. */
+ private lazy val charVarcharCollationMetadata: Map[String, String] =
+ collectCollationMetadata(isCollatedCharVarchar)
Review Comment:
Empty field names break nested CHAR/VARCHAR JSON round trips. The writers
append `.element`/`.key` with a leading dot, while both readers omit that
separator when the base path is empty; for example, `StructField("",
ArrayType(CharType(4, "UTF8_LCASE")))` serializes `.element` and then fails as
an unconsumed path. Could we use the same path-append helper in both Scala and
Python and cover empty-name array/map fields?
##########
sql/api/src/main/scala/org/apache/spark/sql/types/DataType.scala:
##########
@@ -345,21 +368,60 @@ object DataType {
private[sql] def parseDataType(
json: JValue,
fieldPath: String,
- collationsMap: Map[String, String]): DataType = json match {
+ collationsMap: Map[String, String],
+ charVarcharCollationsMap: Map[String, String] = Map.empty): DataType = {
+ val remainingCharVarcharPaths =
mutable.Set.from(charVarcharCollationsMap.keySet)
+ val parsedType = parseDataType(
+ json,
+ fieldPath,
+ collationsMap,
+ charVarcharCollationsMap,
+ remainingCharVarcharPaths)
+ if (remainingCharVarcharPaths.nonEmpty) {
+ throw new SparkIllegalArgumentException(
+ errorClass = "INVALID_JSON_DATA_TYPE_FOR_COLLATIONS",
+ messageParameters = Map("jsonType" -> remainingCharVarcharPaths.min))
Review Comment:
This supplies a metadata path to `INVALID_JSON_DATA_TYPE_FOR_COLLATIONS`,
whose message describes the parameter as a JSON data type. A leftover `typo`
path consequently reports that "the JSON data type is typo"; malformed values
such as `spark.` produce the same misleading framing. Could we add aligned
Scala/Python errors for malformed or unconsumed restoration metadata and
reserve the existing error for entries targeting an actual wrong type?
--
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]
