Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19331#discussion_r140936933
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/PartitioningUtils.scala
---
@@ -470,15 +471,15 @@ object PartitioningUtils {
* Given a collection of [[Literal]]s, resolves possible type conflicts
by up-casting "lower"
* types.
*/
- private def resolveTypeConflicts(literals: Seq[Literal]): Seq[Literal] =
{
+ private def resolveTypeConflicts(literals: Seq[Literal], timeZone:
TimeZone): Seq[Literal] = {
val desiredType = {
val topType =
literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
--- End diff --
I see. Actually, I believe this PR fixed only the (corner) regression
between 2.1.0 and 2.2.0 because, if I understood correctly, we started to
support infer timestamp in partition column from 2.1.0,
[SPARK-17388](https://issues.apache.org/jira/browse/SPARK-17388), and this
corner regression was introduced from 2.2.0
[SPARK-18939](https://issues.apache.org/jira/browse/SPARK-18939) (not tested).
For the issue described above, I think that also should be applied to
`DecimalType`, `DateType` and `TimestampType` which were started to be
supported from [SPARK-17388](https://issues.apache.org/jira/browse/SPARK-17388)
and should strictly be related but orthogonal.
For perfectness, I guess we should port this logic:
https://github.com/apache/spark/blob/183d4cb71fbcbf484fc85d8621e1fe04cbbc8195/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala#L135-L150
because the problem is, `TimestampType`, `DateType` and `DecimalType` can
be upcasted to other types easily by comparing numeric precedence, but of
course with few special handling because we are currently only inferring
decimals when `scale <= 0` (e.g., not 1.1) and castable to a decimal before
trying a double:
https://github.com/apache/spark/blob/04975a68b583a6175f93da52374108e5d4754d9a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/PartitioningUtils.scala#L384-L393
This actually also could a problem of types between `DateType` and
`TimestampType` which should be upcastable (from date to timestamp).
Let me take a closer look and probably make a fix soon.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
