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