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:
us...@infra.apache.org



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

Reply via email to