[GitHub] [spark] Yikf commented on a diff in pull request #41065: [SPARK-43384][SQL] Make `df.show` print a nice string for `MapType`.

["Yikf (via GitHub)" <gi...@apache.org>]

Yikf commented on code in PR #41065:
URL: https://github.com/apache/spark/pull/41065#discussion_r1186641385


##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuiteBase.scala:
##########
@@ -828,7 +828,7 @@ abstract class CastSuiteBase extends SparkFunSuite with ExpressionEvalHelper {
         val ret2 = cast(
           Literal.create(Map("1" -> "a".getBytes, "2" -> null, "3" -> "c".getBytes)),
           StringType)
-        checkEvaluation(ret2, s"${lb}1 -> a, 2 ->${if (legacyCast) "" else " null"}, 3 -> c$rb")
+        checkEvaluation(ret2, s"${lb}1 -> a, 2 -> ${if (legacyCast) "" else "null"}, 3 -> c$rb")

Review Comment:
   I think it's a reasonable change, MapToString has three elements to print, key, value, and separator. that is `k separator v`, this approach is more unified. For Cast, the separator is `" -> "`, and for ToPrettyString, it is `":"`.
   
   This affects only one behavior of the Cast, that is, when v of the first element is an empty string, it was printed as `k ->` before and now is `k -> `.  I think `k -> ` is more reasonable, it is consistent with the other elements except the first element.
   
   For example, map(k1,"",k2,v2), before it's `k1 ->, k2 -> v2`, now it's `k1 -> , k2 -> v2`.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org