ulysses-you commented on code in PR #60078:
URL: https://github.com/apache/spark/pull/60078#discussion_r4477998752
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/physical/partitioning.scala:
##########
@@ -748,17 +748,32 @@ case class KeyedPartitioning(
override def dataType: DataType = IntegerType
/**
- * Prints the layout's contents where the value object would print, which
keeps the plan string as
- * it was before the layout held them. The list is curated rather than the
layout's own fields,
- * for two reasons. `partitionKeys` has to be printed as a `Seq` for
`maxFields` to truncate it,
- * and a partitioning can hold one key per split. And `dataTypes` has its
naming erased, so
- * printing it would put a struct field name into a plan that appears
nowhere in the query. A
- * field the layout grows is therefore a decision here, and the default is
to print it: the plan
- * string is the only place a reader sees why a marked partitioning still
shuffles.
+ * Prints the layout as the partition keys followed by the flags and
origins. The keys are
+ * summarized once they exceed the fields that `maxFields` leaves after the
expressions and the
+ * trailing fields, which are always printed. `dataTypes` is left out
because its naming is
Review Comment:
Fixed in 3900efbe8ee: the scaladoc now says the argument list is truncated
from the end when the expressions and the trailing fields alone exceed the
budget.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/InternalRowComparableWrapper.scala:
##########
@@ -114,6 +125,43 @@ object InternalRowComparableWrapper {
getInternalRowComparableWrapperFactory(partitionExpression.map(_.dataType))(partitionRow)
}
+ /**
+ * Renders a value the way a plan string shows it: by the value's type at
every level. Atomic
+ * values go through the [[Literal]] constructor, whose `toString` formats
dates, timestamps,
+ * decimals and binary; complex values are walked by type, because their own
`toString` shows
+ * internal encodings, hex words or an object identity. The constructor is
used rather than
+ * `Literal.create`, which runs the char/varchar write-side check and can
reject a stored value
+ * while a plan is being printed.
+ */
+ private def render(value: Any, dataType: DataType): String = {
+ if (value == null) {
+ "null"
+ } else {
+ dataType match {
+ case s: StructType =>
+ val r = value.asInstanceOf[InternalRow]
+ s.fields.iterator.zipWithIndex
+ .map { case (field, i) => render(r.get(i, field.dataType),
field.dataType) }
+ .mkString("[", ",", "]")
+ case ArrayType(elementType, _) =>
+ val a = value.asInstanceOf[ArrayData]
+ (0 until a.numElements()).map(i => render(a.get(i, elementType),
elementType))
+ .mkString("[", ",", "]")
+ case MapType(keyType, valueType, _) =>
+ val m = value.asInstanceOf[MapData]
+ (0 until m.numElements()).map { i =>
+ s"${render(m.keyArray().get(i, keyType), keyType)} -> " +
+ render(m.valueArray().get(i, valueType), valueType)
+ }.mkString("{", ",", "}")
+ case udt: UserDefinedType[_] => render(value, udt.sqlType)
+ case _: GeometryType | _: GeographyType =>
Review Comment:
Agreed and removed in 3900efbe8ee. The gate is the hash rather than
isOrderable: a GEOMETRY key is a BinaryView, and KeyedPartitioning.apply fails
in InterpretedHashFunction.hash with a MatchError before any key is built.
MapType keys do work end to end (keyedpartitioning(m#0, [[{k -> 1}], [{k ->
2}]], true, false), with isGrouped and toGrouped passing), so the map branch
stays.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/DistributionSuite.scala:
##########
@@ -17,8 +17,9 @@
package org.apache.spark.sql.catalyst
-import org.apache.spark.SparkFunSuite
+import org.apache.spark.{SparkConf, SparkFunSuite}
/* Implicit conversions */
Review Comment:
Fixed in 3900efbe8ee: JavaSerializer now sits above the implicit-conversions
comment.
--
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]
