Skip to content

Commit de73cda

Browse files
luigidellaquilaluigidellaquila
authored
SQL: make date format functions more strict (#112140)
1 parent 554eb4f commit de73cda

File tree

5 files changed

+79
-50
lines changed

5 files changed

+79
-50
lines changed

‎x-pack/plugin/sql/qa/server/src/main/resources/datetime.csv-spec‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1922,3 +1922,21 @@ SELECT hire_date FROM test_emp WHERE '2005-12-14T00:00:00.000Z'::datetime BETWEE
19221922
------------------------
19231923

19241924
;
1925+
1926+
// checking regressions after https://github.com/elastic/elasticsearch/pull/110222
1927+
selectDateFunctionsCldr
1928+
schema::month:s|day_of_week:s|day:s|week:s|ad:s|day_of_week2:s|month2:s|ad2:s
1929+
SELECT DATE_FORMAT('2020-04-05T11:22:33.123Z'::date, '%M') AS month,
1930+
DATE_FORMAT('2020-04-05T11:22:33.123Z'::date, '%W') AS day_of_week,
1931+
DATE_FORMAT('2020-04-05T11:22:33.123Z'::date, '%w') AS day,
1932+
DATE_FORMAT('2020-04-05T11:22:33.123Z'::date, '%v') AS week,
1933+
DATETIME_FORMAT('2020-04-05T11:22:33.123Z'::date, 'G') AS ad,
1934+
TO_CHAR('2020-04-05T11:22:33.123Z'::date, 'Day') AS day_of_week2,
1935+
TO_CHAR('2020-04-05T11:22:33.123Z'::date, 'Month') AS month2,
1936+
TO_CHAR('2020-04-05T11:22:33.123Z'::date, 'BC') AS ad2;
1937+
1938+
month | day_of_week | day | week | ad | day_of_week2 | month2 | ad2
1939+
--------------+-----------------+---------+---------+--------+----------------+--------------+------------
1940+
April | Sunday | 0 | 14 | AD | "Sunday " | "April " | AD
1941+
;
1942+

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateFormatter.java‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class DateFormatter {
4242
new Builder().pattern("%e").javaPattern("d").build(),
4343
new Builder().pattern("%f")
4444
.javaPattern("n")
45-
.additionalMapper(s -> String.format(Locale.ROOT, "%06d", Math.round(Integer.parseInt(s) / 1000.0)))
45+
.additionalMapper(s -> String.format(Locale.ENGLISH, "%06d", Math.round(Integer.parseInt(s) / 1000.0)))
4646
.build(),
4747
new Builder().pattern("%H").javaPattern("HH").build(),
4848
new Builder().pattern("%h").javaPattern("hh").build(),
@@ -59,19 +59,28 @@ class DateFormatter {
5959
new Builder().pattern("%s").javaPattern("ss").build(),
6060
new Builder().pattern("%T").javaPattern("HH:mm:ss").build(),
6161
new Builder().pattern("%U")
62-
.javaFormat(t -> String.format(Locale.ROOT, "%02d", t.get(WeekFields.of(DayOfWeek.SUNDAY, 7).weekOfYear())))
62+
.javaFormat(t -> String.format(Locale.ENGLISH, "%02d", t.get(WeekFields.of(DayOfWeek.SUNDAY, 7).weekOfYear())))
6363
.build(),
64-
new Builder().pattern("%u").javaFormat(t -> String.format(Locale.ROOT, "%02d", t.get(WeekFields.ISO.weekOfYear()))).build(),
64+
new Builder().pattern("%u")
65+
.javaFormat(t -> String.format(Locale.ENGLISH, "%02d", t.get(WeekFields.of(DayOfWeek.MONDAY, 4).weekOfYear())))
66+
.build(),
67+
6568
new Builder().pattern("%V")
66-
.javaFormat(t -> String.format(Locale.ROOT, "%02d", t.get(WeekFields.of(DayOfWeek.SUNDAY, 7).weekOfWeekBasedYear())))
69+
.javaFormat(t -> String.format(Locale.ENGLISH, "%02d", t.get(WeekFields.of(DayOfWeek.SUNDAY, 7).weekOfWeekBasedYear())))
70+
.build(),
71+
new Builder().pattern("%v")
72+
.javaFormat(t -> String.format(Locale.ENGLISH, "%02d", t.get(WeekFields.of(DayOfWeek.MONDAY, 4).weekOfWeekBasedYear())))
6773
.build(),
68-
new Builder().pattern("%v").javaPattern("ww").build(),
6974
new Builder().pattern("%W").javaPattern("EEEE").build(),
70-
new Builder().pattern("%w").javaPattern("e").additionalMapper(s -> Integer.parseInt(s) == 7 ? String.valueOf(0) : s).build(),
75+
new Builder().pattern("%w")
76+
.javaFormat(t -> String.format(Locale.ENGLISH, "%01d", t.get(WeekFields.of(DayOfWeek.SUNDAY, 7).dayOfWeek()) - 1))
77+
.build(),
7178
new Builder().pattern("%X")
72-
.javaFormat(t -> String.format(Locale.ROOT, "%04d", t.get(WeekFields.of(DayOfWeek.SUNDAY, 7).weekBasedYear())))
79+
.javaFormat(t -> String.format(Locale.ENGLISH, "%04d", t.get(WeekFields.of(DayOfWeek.SUNDAY, 7).weekBasedYear())))
80+
.build(),
81+
new Builder().pattern("%x")
82+
.javaFormat(t -> String.format(Locale.ENGLISH, "%04d", t.get(WeekFields.of(DayOfWeek.MONDAY, 7).weekBasedYear())))
7383
.build(),
74-
new Builder().pattern("%x").javaPattern("Y").build(),
7584
new Builder().pattern("%Y").javaPattern("yyyy").build(),
7685
new Builder().pattern("%y").javaPattern("yy").build()
7786
);
@@ -162,7 +171,7 @@ private Builder pattern(String pattern) {
162171
}
163172

164173
private Builder javaPattern(String javaPattern) {
165-
this.javaFormat = temporalAccessor -> DateTimeFormatter.ofPattern(javaPattern, Locale.ROOT).format(temporalAccessor);
174+
this.javaFormat = temporalAccessor -> DateTimeFormatter.ofPattern(javaPattern, Locale.ENGLISH).format(temporalAccessor);
166175
return this;
167176
}
168177

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeFormatProcessor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ protected Function<TemporalAccessor, String> formatterFor(String pattern) {
8585
return null;
8686
}
8787
final String javaPattern = msToJavaPattern(pattern);
88-
return DateTimeFormatter.ofPattern(javaPattern, Locale.ROOT)::format;
88+
return DateTimeFormatter.ofPattern(javaPattern, Locale.ENGLISH)::format;
8989
}
9090
},
9191
DATE_FORMAT {
@@ -97,7 +97,7 @@ protected Function<TemporalAccessor, String> formatterFor(String pattern) {
9797
DATE_TIME_FORMAT {
9898
@Override
9999
protected Function<TemporalAccessor, String> formatterFor(String pattern) {
100-
return DateTimeFormatter.ofPattern(pattern, Locale.ROOT)::format;
100+
return DateTimeFormatter.ofPattern(pattern, Locale.ENGLISH)::format;
101101
}
102102
},
103103
TO_CHAR {

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/NamedDateTimeProcessor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ public final String extract(ZonedDateTime millis, String tzId) {
4040
}
4141

4242
public static final String NAME = "ndt";
43-
private static final DateTimeFormatter DAY_NAME_FORMATTER = DateTimeFormatter.ofPattern("EEEE", Locale.ROOT);
44-
private static final DateTimeFormatter MONTH_NAME_FORMATTER = DateTimeFormatter.ofPattern("MMMM", Locale.ROOT);
43+
private static final DateTimeFormatter DAY_NAME_FORMATTER = DateTimeFormatter.ofPattern("EEEE", Locale.ENGLISH);
44+
private static final DateTimeFormatter MONTH_NAME_FORMATTER = DateTimeFormatter.ofPattern("MMMM", Locale.ENGLISH);
4545

4646
private final NameExtractor extractor;
4747

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/ToCharFormatter.java‎

Lines changed: 39 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class ToCharFormatter {
4141
of("HH12").formatFn("hh").numeric(),
4242
of("HH24").formatFn("HH").numeric(),
4343
of("MI").formatFn("mm").numeric(),
44-
of("SS").formatFn("s", x -> String.format(Locale.ROOT, "%02d", parseInt(x))).numeric(),
44+
of("SS").formatFn("s", x -> String.format(Locale.ENGLISH, "%02d", parseInt(x))).numeric(),
4545
of("MS").formatFn("n", nano -> firstDigitsOfNanos(nano, 3)).numericWithLeadingZeros(),
4646
of("US").formatFn("n", nano -> firstDigitsOfNanos(nano, 6)).numericWithLeadingZeros(),
4747
of("FF1").formatFn("n", nano -> firstDigitsOfNanos(nano, 1)).numericWithLeadingZeros(),
@@ -52,14 +52,14 @@ class ToCharFormatter {
5252
of("FF6").formatFn("n", nano -> firstDigitsOfNanos(nano, 6)).numericWithLeadingZeros(),
5353
of("SSSSS").formatFn("A", milliSecondOfDay -> String.valueOf(parseInt(milliSecondOfDay) / 1000)).numeric(),
5454
of("SSSS").formatFn("A", milliSecondOfDay -> String.valueOf(parseInt(milliSecondOfDay) / 1000)).numeric(),
55-
of("AM").formatFn("a", x -> x.toUpperCase(Locale.ROOT)).text(),
56-
of("am").formatFn("a", x -> x.toLowerCase(Locale.ROOT)).text(),
57-
of("PM").formatFn("a", x -> x.toUpperCase(Locale.ROOT)).text(),
58-
of("pm").formatFn("a", x -> x.toLowerCase(Locale.ROOT)).text(),
55+
of("AM").formatFn("a", x -> x.toUpperCase(Locale.ENGLISH)).text(),
56+
of("am").formatFn("a", x -> x.toLowerCase(Locale.ENGLISH)).text(),
57+
of("PM").formatFn("a", x -> x.toUpperCase(Locale.ENGLISH)).text(),
58+
of("pm").formatFn("a", x -> x.toLowerCase(Locale.ENGLISH)).text(),
5959
of("A.M.").formatFn("a", x -> x.charAt(0) + "." + x.charAt(1) + ".").text(),
60-
of("a.m.").formatFn("a", x -> (x.charAt(0) + "." + x.charAt(1) + ".").toLowerCase(Locale.ROOT)).text(),
60+
of("a.m.").formatFn("a", x -> (x.charAt(0) + "." + x.charAt(1) + ".").toLowerCase(Locale.ENGLISH)).text(),
6161
of("P.M.").formatFn("a", x -> x.charAt(0) + "." + x.charAt(1) + ".").text(),
62-
of("p.m.").formatFn("a", x -> (x.charAt(0) + "." + x.charAt(1) + ".").toLowerCase(Locale.ROOT)).text(),
62+
of("p.m.").formatFn("a", x -> (x.charAt(0) + "." + x.charAt(1) + ".").toLowerCase(Locale.ENGLISH)).text(),
6363
of("Y,YYY").formatFn("yyyy", year -> year.charAt(0) + "," + year.substring(1)).numericWithLeadingZeros(),
6464
of("YYYY").formatFn("yyyy").numeric(),
6565
of("YYY").formatFn("yyyy", year -> year.substring(1)).numeric(),
@@ -70,51 +70,53 @@ class ToCharFormatter {
7070
of("IY").formatFn(t -> lastNCharacter(absoluteWeekBasedYear(t), 2)).numeric(),
7171
of("I").formatFn(t -> lastNCharacter(absoluteWeekBasedYear(t), 1)).numeric(),
7272
of("BC").formatFn("G").text(),
73-
of("bc").formatFn("G", x -> x.toLowerCase(Locale.ROOT)).text(),
73+
of("bc").formatFn("G", x -> x.toLowerCase(Locale.ENGLISH)).text(),
7474
of("AD").formatFn("G").text(),
75-
of("ad").formatFn("G", x -> x.toLowerCase(Locale.ROOT)).text(),
75+
of("ad").formatFn("G", x -> x.toLowerCase(Locale.ENGLISH)).text(),
7676
of("B.C.").formatFn("G", x -> x.charAt(0) + "." + x.charAt(1) + ".").text(),
77-
of("b.c.").formatFn("G", x -> (x.charAt(0) + "." + x.charAt(1) + ".").toLowerCase(Locale.ROOT)).text(),
77+
of("b.c.").formatFn("G", x -> (x.charAt(0) + "." + x.charAt(1) + ".").toLowerCase(Locale.ENGLISH)).text(),
7878
of("A.D.").formatFn("G", x -> x.charAt(0) + "." + x.charAt(1) + ".").text(),
79-
of("a.d.").formatFn("G", x -> (x.charAt(0) + "." + x.charAt(1) + ".").toLowerCase(Locale.ROOT)).text(),
80-
of("MONTH").formatFn("MMMM", x -> String.format(Locale.ROOT, "%-9s", x.toUpperCase(Locale.ROOT))).text(),
81-
of("Month").formatFn("MMMM", x -> String.format(Locale.ROOT, "%-9s", x)).text(),
82-
of("month").formatFn("MMMM", x -> String.format(Locale.ROOT, "%-9s", x.toLowerCase(Locale.ROOT))).text(),
83-
of("MON").formatFn("MMM", x -> x.toUpperCase(Locale.ROOT)).text(),
79+
of("a.d.").formatFn("G", x -> (x.charAt(0) + "." + x.charAt(1) + ".").toLowerCase(Locale.ENGLISH)).text(),
80+
of("MONTH").formatFn("MMMM", x -> String.format(Locale.ENGLISH, "%-9s", x.toUpperCase(Locale.ENGLISH))).text(),
81+
of("Month").formatFn("MMMM", x -> String.format(Locale.ENGLISH, "%-9s", x)).text(),
82+
of("month").formatFn("MMMM", x -> String.format(Locale.ENGLISH, "%-9s", x.toLowerCase(Locale.ENGLISH))).text(),
83+
of("MON").formatFn("MMM", x -> x.toUpperCase(Locale.ENGLISH)).text(),
8484
of("Mon").formatFn("MMM").text(),
85-
of("mon").formatFn("MMM", x -> x.toLowerCase(Locale.ROOT)).text(),
85+
of("mon").formatFn("MMM", x -> x.toLowerCase(Locale.ENGLISH)).text(),
8686
of("MM").formatFn("MM").numeric(),
87-
of("DAY").formatFn("EEEE", x -> String.format(Locale.ROOT, "%-9s", x.toUpperCase(Locale.ROOT))).text(),
88-
of("Day").formatFn("EEEE", x -> String.format(Locale.ROOT, "%-9s", x)).text(),
89-
of("day").formatFn("EEEE", x -> String.format(Locale.ROOT, "%-9s", x.toLowerCase(Locale.ROOT))).text(),
90-
of("DY").formatFn("E", x -> x.toUpperCase(Locale.ROOT)).text(),
87+
of("DAY").formatFn("EEEE", x -> String.format(Locale.ENGLISH, "%-9s", x.toUpperCase(Locale.ENGLISH))).text(),
88+
of("Day").formatFn("EEEE", x -> String.format(Locale.ENGLISH, "%-9s", x)).text(),
89+
of("day").formatFn("EEEE", x -> String.format(Locale.ENGLISH, "%-9s", x.toLowerCase(Locale.ENGLISH))).text(),
90+
of("DY").formatFn("E", x -> x.toUpperCase(Locale.ENGLISH)).text(),
9191
of("Dy").formatFn("E").text(),
92-
of("dy").formatFn("E", x -> x.toLowerCase(Locale.ROOT)).text(),
92+
of("dy").formatFn("E", x -> x.toLowerCase(Locale.ENGLISH)).text(),
9393
of("DDD").formatFn("DDD").numeric(),
9494
of("IDDD").formatFn(
9595
t -> String.format(
96-
Locale.ROOT,
96+
Locale.ENGLISH,
9797
"%03d",
9898
(t.get(WeekFields.ISO.weekOfWeekBasedYear()) - 1) * 7 + t.get(ChronoField.DAY_OF_WEEK)
9999
)
100100
).numeric(),
101-
of("DD").formatFn("d", x -> String.format(Locale.ROOT, "%02d", parseInt(x))).numeric(),
101+
of("DD").formatFn("d", x -> String.format(Locale.ENGLISH, "%02d", parseInt(x))).numeric(),
102102
of("ID").formatFn(t -> String.valueOf(t.get(ChronoField.DAY_OF_WEEK))).numeric(),
103103
of("D").formatFn(t -> String.valueOf(t.get(WeekFields.SUNDAY_START.dayOfWeek()))).numeric(),
104104
of("W").formatFn(t -> String.valueOf(t.get(ChronoField.ALIGNED_WEEK_OF_MONTH))).numeric(),
105-
of("WW").formatFn(t -> String.format(Locale.ROOT, "%02d", t.get(ChronoField.ALIGNED_WEEK_OF_YEAR))).numeric(),
106-
of("IW").formatFn(t -> String.format(Locale.ROOT, "%02d", t.get(WeekFields.ISO.weekOfWeekBasedYear()))).numeric(),
105+
of("WW").formatFn(t -> String.format(Locale.ENGLISH, "%02d", t.get(ChronoField.ALIGNED_WEEK_OF_YEAR))).numeric(),
106+
of("IW").formatFn(t -> String.format(Locale.ENGLISH, "%02d", t.get(WeekFields.ISO.weekOfWeekBasedYear()))).numeric(),
107107
of("CC").formatFn(t -> {
108108
int century = yearToCentury(t.get(ChronoField.YEAR));
109-
return String.format(Locale.ROOT, century < 0 ? "%03d" : "%02d", century);
109+
return String.format(Locale.ENGLISH, century < 0 ? "%03d" : "%02d", century);
110110
}).numeric(),
111111
of("J").formatFn(t -> String.valueOf(t.getLong(JulianFields.JULIAN_DAY))).numeric(),
112112
of("Q").formatFn("Q").numeric(),
113-
of("RM").formatFn("MM", month -> String.format(Locale.ROOT, "%-4s", monthToRoman(parseInt(month)))).text(),
114-
of("rm").formatFn("MM", month -> String.format(Locale.ROOT, "%-4s", monthToRoman(parseInt(month)).toLowerCase(Locale.ROOT)))
115-
.text(),
113+
of("RM").formatFn("MM", month -> String.format(Locale.ENGLISH, "%-4s", monthToRoman(parseInt(month)))).text(),
114+
of("rm").formatFn(
115+
"MM",
116+
month -> String.format(Locale.ENGLISH, "%-4s", monthToRoman(parseInt(month)).toLowerCase(Locale.ENGLISH))
117+
).text(),
116118
of("TZ").formatFn(ToCharFormatter::zoneAbbreviationOf).text(),
117-
of("tz").formatFn(t -> zoneAbbreviationOf(t).toLowerCase(Locale.ROOT)).text(),
119+
of("tz").formatFn(t -> zoneAbbreviationOf(t).toLowerCase(Locale.ENGLISH)).text(),
118120
of("TZH").acceptsLowercase(false).formatFn("ZZ", s -> s.substring(0, 3)).text(),
119121
of("TZM").acceptsLowercase(false).formatFn("ZZ", s -> lastNCharacter(s, 2)).text(),
120122
of("OF").acceptsLowercase(false).formatFn("ZZZZZ", ToCharFormatter::formatOffset).offset()
@@ -127,7 +129,7 @@ class ToCharFormatter {
127129
// also index the lower case version of the patterns if accepted
128130
for (ToCharFormatter formatter : formatters) {
129131
if (formatter.acceptsLowercase) {
130-
formatterMap.putIfAbsent(formatter.pattern.toLowerCase(Locale.ROOT), formatter);
132+
formatterMap.putIfAbsent(formatter.pattern.toLowerCase(Locale.ENGLISH), formatter);
131133
}
132134
}
133135
FORMATTER_MAP = formatterMap;
@@ -274,8 +276,8 @@ private static String appendOrdinalSuffix(String defaultSuffix, String s) {
274276
// the Y,YYY pattern might can cause problems with the parsing, but thankfully the last 3
275277
// characters is enough to calculate the suffix
276278
int i = parseInt(lastNCharacter(s, 3));
277-
final boolean upperCase = defaultSuffix.equals(defaultSuffix.toUpperCase(Locale.ROOT));
278-
return s + (upperCase ? ordinalSuffix(i).toUpperCase(Locale.ROOT) : ordinalSuffix(i));
279+
final boolean upperCase = defaultSuffix.equals(defaultSuffix.toUpperCase(Locale.ENGLISH));
280+
return s + (upperCase ? ordinalSuffix(i).toUpperCase(Locale.ENGLISH) : ordinalSuffix(i));
279281
} catch (NumberFormatException ex) {
280282
return s + defaultSuffix;
281283
}
@@ -312,19 +314,19 @@ private static String removeLeadingZerosFromOffset(String offset) {
312314
private static String absoluteWeekBasedYear(TemporalAccessor t) {
313315
int year = t.get(IsoFields.WEEK_BASED_YEAR);
314316
year = year > 0 ? year : -(year - 1);
315-
return String.format(Locale.ROOT, "%04d", year);
317+
return String.format(Locale.ENGLISH, "%04d", year);
316318
}
317319

318320
private static String firstDigitsOfNanos(String nano, int digits) {
319-
return String.format(Locale.ROOT, "%09d", parseInt(nano)).substring(0, digits);
321+
return String.format(Locale.ENGLISH, "%09d", parseInt(nano)).substring(0, digits);
320322
}
321323

322324
private static String lastNCharacter(String s, int n) {
323325
return s.substring(Math.max(0, s.length() - n));
324326
}
325327

326328
private static String zoneAbbreviationOf(TemporalAccessor temporalAccessor) {
327-
String zone = ZoneId.from(temporalAccessor).getDisplayName(TextStyle.SHORT, Locale.ROOT);
329+
String zone = ZoneId.from(temporalAccessor).getDisplayName(TextStyle.SHORT, Locale.ENGLISH);
328330
return "Z".equals(zone) ? "UTC" : zone;
329331
}
330332

@@ -344,7 +346,7 @@ public Builder formatFn(final String javaPattern) {
344346

345347
public Builder formatFn(final String javaPattern, final Function<String, String> additionalMapper) {
346348
this.formatFn = temporalAccessor -> {
347-
String formatted = DateTimeFormatter.ofPattern(javaPattern != null ? javaPattern : "'" + pattern + "'", Locale.ROOT)
349+
String formatted = DateTimeFormatter.ofPattern(javaPattern != null ? javaPattern : "'" + pattern + "'", Locale.ENGLISH)
348350
.format(temporalAccessor);
349351
return additionalMapper == null ? formatted : additionalMapper.apply(formatted);
350352
};

0 commit comments

Comments
 (0)