YiwenWu commented on code in PR #3680:
URL: https://github.com/apache/calcite/pull/3680#discussion_r1484932837


##########
core/src/main/java/org/apache/calcite/util/format/FormatElementEnum.java:
##########
@@ -57,8 +59,17 @@ public enum FormatElementEnum implements FormatElement {
   },
   DAY("EEEE", "The full weekday name") {
     @Override public void format(StringBuilder sb, Date date) {
-      final Work work = Work.get();
-      sb.append(work.eeeeFormat.format(date));
+      final Calendar calendar = Work.get().calendar;
+      calendar.setTime(date);
+      // The Calendar and SimpleDateFormatter do not seem to give correct 
results
+      // for the day of the week prior to the Julian to Gregorian date change.
+      // So we resort to using a LocalDate representation.
+      LocalDate ld =

Review Comment:
   Can use java.sql.Date directly?
   `((java.sql.Date) date).toLocalDate()`
   



##########
core/src/main/java/org/apache/calcite/util/format/FormatElementEnum.java:
##########
@@ -281,9 +301,9 @@ static Work get() {
 
     /** Uses Locale.US instead of Locale.ROOT to fix formatting in Java 11 */
     final DateFormat eeeeFormat = new SimpleDateFormat(DAY.javaFmt, Locale.US);
-    final DateFormat eeeFormat = new SimpleDateFormat(DY.javaFmt, Locale.ROOT);
-    final DateFormat mmmFormat = new SimpleDateFormat(MON.javaFmt, 
Locale.ROOT);
-    final DateFormat mmmmFormat = new SimpleDateFormat(MONTH.javaFmt, 
Locale.ROOT);
+    final DateFormat eeeFormat = new SimpleDateFormat(DY.javaFmt, Locale.US);

Review Comment:
   Why use Locale.US?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to