[GitHub] [spark] cloud-fan commented on a change in pull request #28674: [SPARK-31868][SQL] Restore the parsing behaviour week-based-year for 2.4

[GitBox <gi...@apache.org>]

cloud-fan commented on a change in pull request #28674:
URL: https://github.com/apache/spark/pull/28674#discussion_r433297235



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeFormatterHelper.scala
##########
@@ -31,26 +31,52 @@ import org.apache.spark.sql.internal.SQLConf
 import org.apache.spark.sql.internal.SQLConf.LegacyBehaviorPolicy._
 
 trait DateTimeFormatterHelper {
-  private def getOrDefault(accessor: TemporalAccessor, field: ChronoField, default: Int): Int = {
+
+  private def getOrDefault(accessor: TemporalAccessor, field: TemporalField, default: Int): Int = {
     if (accessor.isSupported(field)) {
       accessor.get(field)
     } else {
       default
     }
   }
 
-  protected def toLocalDate(accessor: TemporalAccessor): LocalDate = {
+  private def mayNonWeekBased(accessor: TemporalAccessor): Boolean = {
+    val has = accessor.isSupported _
+    has(ChronoField.YEAR) || has(ChronoField.MONTH_OF_YEAR) || has(ChronoField.DAY_OF_MONTH)
+  }
+
+  private def mayWeekBased(accessor: TemporalAccessor, wf: WeekFields): Boolean = {
+    val has = accessor.isSupported _
+    has(wf.weekBasedYear) || has(wf.weekOfMonth) || has(wf.weekOfWeekBasedYear) || has(wf.dayOfWeek)
+  }
+
+  @throws[DateTimeException]
+  protected def toLocalDate(accessor: TemporalAccessor, locale: Locale): LocalDate = {
     val localDate = accessor.query(TemporalQueries.localDate())
-    // If all the date fields are specified, return the local date directly.
+    // If all the date fields are resolved(yMd or Ywu), return the local date directly.
     if (localDate != null) return localDate
 
-    // Users may want to parse only a few datetime fields from a string and extract these fields
-    // later, and we should provide default values for missing fields.
-    // To be compatible with Spark 2.4, we pick 1970 as the default value of year.
-    val year = getOrDefault(accessor, ChronoField.YEAR, 1970)
-    val month = getOrDefault(accessor, ChronoField.MONTH_OF_YEAR, 1)
-    val day = getOrDefault(accessor, ChronoField.DAY_OF_MONTH, 1)
-    LocalDate.of(year, month, day)
+    val weekFields = WeekFields.of(locale)
+    val weekBased = mayWeekBased(accessor, weekFields)
+    if (weekBased && mayNonWeekBased(accessor)) {
+      throw new DateTimeException(
+        s"Can not mix week-based and non-week-based date fields together for parsing dates")

Review comment:
       should this happen when we create the formatter?




----------------------------------------------------------------
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.

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