SQL: Fix the inconsistent behaviour of ISO_WEEK_YEAR()

The `SELECT ISO_WEEK_OF_YEAR(a) AS x FROM test WHERE x=4` query returned
with `x=3` results because the `ISO_WEEK_YEAR(a)` in the WHERE clause
that turns into a script query and the `ISO_WEEK_YEAR(a)` in the projections
that turns into a post-processing on top of the Query DSL results execute
different code to calculate the result.

This change unifies the different code paths and results in a single method
being responsible for the actual calculation.

Note: this change impacts the way how all the `DateTimeFunction`s that
do the field extraction from a date get translated into a script query.

Fixes part of elastic#67872

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

@@ -131,6 +131,32 @@ SELECT WEEK(birth_date) week, birth_date FROM test_emp ORDER BY WEEK(birth_date)
 44             |1961-11-02T00:00:00.000Z
 ;
 
+isoWeekOfYear
+schema::birth_date:ts|iso_week:i|week:i
+SELECT birth_date, IW(birth_date) iso_week, WEEK(birth_date) week FROM test_emp WHERE IW(birth_date) < 8 ORDER BY iso_week;
+
+       birth_date       |   iso_week    |     week      
+------------------------+---------------+---------------
+1953-01-07T00:00:00.000Z|2              |2              
+1955-01-21T00:00:00.000Z|3              |4              
+1953-01-23T00:00:00.000Z|4              |4              
+1958-01-21T00:00:00.000Z|4              |4              
+1959-01-27T00:00:00.000Z|5              |5              
+1956-02-12T00:00:00.000Z|6              |7              
+1953-02-08T00:00:00.000Z|6              |7              
+1960-02-20T00:00:00.000Z|7              |8              
+;
+
+isoWeekOfYearFilterEquality
+SELECT ISO_WEEK_OF_YEAR(CONCAT(CONCAT('2021-01-22T14:26:06.', (salary % 2)::text), 'Z')::datetime) AS iso_week 
+FROM test_emp WHERE iso_week = 3 LIMIT 2;
+
+   iso_week:i  
+---------------
+3              
+3              
+;
+
 weekOfYearVsIsoWeekOfYearEdgeCases
 SELECT ISO_WEEK_OF_YEAR('2005-01-01T00:00:00.000Z'::datetime) AS "isow2005", WEEK('2005-01-01T00:00:00.000Z'::datetime) AS "w2005",
 ISO_WEEK_OF_YEAR('2007-12-31T00:00:00.000Z'::datetime) AS "isow2007", WEEK('2007-12-31T00:00:00.000Z'::datetime) AS "w2007";
@@ -1303,24 +1329,25 @@ null           |null           |10
 dateTimeAggByIsoWeekOfYear
 SELECT IW(birth_date) iso_week, WEEK(birth_date) week FROM test_emp WHERE IW(birth_date) < 20 GROUP BY iso_week, week ORDER BY iso_week;
 
- iso_week:i    |    week:i      
+   iso_week:i  |     week:i    
 ---------------+---------------
-1              |2              
+2              |2              
 3              |4              
 4              |4              
-4              |5              
+5              |5              
 6              |7              
-7              |7              
+7              |8              
 8              |8              
 8              |9              
 9              |9              
 10             |11             
 12             |12             
 14             |14             
-14             |15             
+15             |15             
 15             |16             
 16             |16             
-16             |17             
+17             |17             
+17             |18             
 18             |18             
 ;
 

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

@@ -16,9 +16,6 @@
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeProcessor.DateTimeExtractor;
 
 import java.time.ZoneId;
-import java.time.ZonedDateTime;
-import java.time.temporal.ChronoField;
-import java.time.temporal.Temporal;
 
 import static org.elasticsearch.xpack.ql.expression.gen.script.ParamsBuilder.paramsBuilder;
 
@@ -31,24 +28,15 @@ public abstract class DateTimeFunction extends BaseDateTimeFunction {
         this.extractor = extractor;
     }
 
-    public static Integer dateTimeChrono(ZonedDateTime dateTime, String tzId, String chronoName) {
-        ZonedDateTime zdt = dateTime.withZoneSameInstant(ZoneId.of(tzId));
-        return dateTimeChrono(zdt, ChronoField.valueOf(chronoName));
-    }
-
-    protected static Integer dateTimeChrono(Temporal dateTime, ChronoField field) {
-        return Integer.valueOf(dateTime.get(field));
-    }
-
     @Override
     public ScriptTemplate asScript() {
         ParamsBuilder params = paramsBuilder();
 
         ScriptTemplate script = super.asScript();
-        String template = formatTemplate("{sql}.dateTimeChrono(" + script.template() + ", {}, {})");
+        String template = formatTemplate("{sql}.dateTimeExtract(" + script.template() + ", {}, {})");
         params.script(script.params())
               .variable(zoneId().getId())
-              .variable(extractor.chronoField().name());
+              .variable(extractor.name());
 
         return new ScriptTemplate(template, params.build(), dataType());
     }

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

@@ -10,11 +10,12 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 
 import java.io.IOException;
-import java.time.OffsetTime;
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
 import java.time.temporal.ChronoField;
-import java.time.temporal.WeekFields;
+import java.time.temporal.IsoFields;
+import java.time.temporal.Temporal;
+import java.time.temporal.TemporalField;
 import java.util.Objects;
 
 public class DateTimeProcessor extends BaseDateTimeProcessor {
@@ -28,30 +29,18 @@ public enum DateTimeExtractor {
         MINUTE_OF_HOUR(ChronoField.MINUTE_OF_HOUR),
         MONTH_OF_YEAR(ChronoField.MONTH_OF_YEAR),
         SECOND_OF_MINUTE(ChronoField.SECOND_OF_MINUTE),
-        ISO_WEEK_OF_YEAR(ChronoField.ALIGNED_WEEK_OF_YEAR),
+        ISO_WEEK_OF_YEAR(IsoFields.WEEK_OF_WEEK_BASED_YEAR),
         YEAR(ChronoField.YEAR);
 
-        private final ChronoField field;
+        private final TemporalField field;
 
-        DateTimeExtractor(ChronoField field) {
+        DateTimeExtractor(TemporalField field) {
             this.field = field;
         }
-
-        public int extract(ZonedDateTime dt) {
-            if (field == ChronoField.ALIGNED_WEEK_OF_YEAR) {
-                return dt.get(WeekFields.ISO.weekOfWeekBasedYear());
-            } else {
-                return dt.get(field);
-            }
-        }
-
-        public int extract(OffsetTime time) {
+
+        public int extract(Temporal time) {
             return time.get(field);
         }
-
-        public ChronoField chronoField() {
-            return field;
-        }
     }
 
     public static final String NAME = "dt";
@@ -86,6 +75,15 @@ public Object doProcess(ZonedDateTime dateTime) {
         return extractor.extract(dateTime);
     }
 
+    public static Integer doProcess(ZonedDateTime dateTime, String tzId, String extractorName) {
+        ZonedDateTime zdt = dateTime.withZoneSameInstant(ZoneId.of(tzId));
+        return doProcess(zdt, extractorName);
+    }
+
+    protected static Integer doProcess(Temporal dateTime, String extractorName) {
+        return DateTimeExtractor.valueOf(extractorName).extract(dateTime);
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(extractor, zoneId());

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

@@ -12,23 +12,16 @@
 import org.elasticsearch.xpack.ql.tree.Source;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeProcessor.DateTimeExtractor;
 
-import java.time.OffsetTime;
 import java.time.ZoneId;
-import java.time.temporal.ChronoField;
 
 import static org.elasticsearch.xpack.sql.expression.SqlTypeResolutions.isDateOrTime;
-import static org.elasticsearch.xpack.sql.util.DateUtils.asTimeAtZone;
 
 public abstract class TimeFunction extends DateTimeFunction {
 
     TimeFunction(Source source, Expression field, ZoneId zoneId, DateTimeExtractor extractor) {
         super(source, field, zoneId, extractor);
     }
 
-    public static Integer dateTimeChrono(OffsetTime time, String tzId, String chronoName) {
-        return dateTimeChrono(asTimeAtZone(time, ZoneId.of(tzId)), ChronoField.valueOf(chronoName));
-    }
-
     @Override
     protected TypeResolution resolveType() {
         return isDateOrTime(field(), sourceText(), Expressions.ParamOrdinal.DEFAULT);

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

@@ -43,6 +43,10 @@ public Object process(Object input) {
     private Object doProcess(OffsetTime time) {
         return extractor().extract(time);
     }
+
+    public static Integer doProcess(OffsetTime dateTime, String tzId, String extractorName) {
+        return DateTimeProcessor.doProcess(asTimeAtZone(dateTime, ZoneId.of(tzId)), extractorName);
+    }
 
     @Override
     public int hashCode() {

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/whitelist/InternalSqlScriptUtils.java

@@ -15,13 +15,13 @@
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateDiffProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DatePartProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeFormatProcessor.Formatter;
-import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeFunction;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeParseProcessor.Parser;
+import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTruncProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.NamedDateTimeProcessor.NameExtractor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.NonIsoDateTimeProcessor.NonIsoDateTimeExtractor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.QuarterProcessor;
-import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.TimeFunction;
+import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.TimeProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.geo.GeoProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.geo.StDistanceProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.geo.StWkttosqlProcessor;
@@ -221,14 +221,14 @@ public static Number tan(Number value) {
     //
     // Date/Time functions
     //
-    public static Integer dateTimeChrono(Object dateTime, String tzId, String chronoName) {
-        if (dateTime == null || tzId == null || chronoName == null) {
+    public static Integer dateTimeExtract(Object dateTime, String tzId, String extractorName) {
+        if (dateTime == null || tzId == null || extractorName == null) {
             return null;
         }
         if (dateTime instanceof OffsetTime) {
-            return TimeFunction.dateTimeChrono((OffsetTime) dateTime, tzId, chronoName);
+            return TimeProcessor.doProcess((OffsetTime) dateTime, tzId, extractorName);
         }
-        return DateTimeFunction.dateTimeChrono(asDateTime(dateTime), tzId, chronoName);
+        return DateTimeProcessor.doProcess(asDateTime(dateTime), tzId, extractorName);
     }
 
     public static String dayName(Object dateTime, String tzId) {

x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt

@@ -124,7 +124,7 @@ class org.elasticsearch.xpack.sql.expression.function.scalar.whitelist.InternalS
 #
 # Date/Time functions
 #
-  Integer dateTimeChrono(Object, String, String)
+  Integer dateTimeExtract(Object, String, String)
   String  dayName(Object, String)
   Integer dayOfWeek(Object, String)
   String  monthName(Object, String)

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java

@@ -1214,7 +1214,7 @@ public void testTranslateRoundWithOneParameter() {
         assertNotNull(groupingContext);
         ScriptTemplate scriptTemplate = groupingContext.tail.script();
         assertEquals(
-                "InternalSqlScriptUtils.round(InternalSqlScriptUtils.dateTimeChrono(InternalQlScriptUtils.docValue(doc,params.v0), "
+                "InternalSqlScriptUtils.round(InternalSqlScriptUtils.dateTimeExtract(InternalQlScriptUtils.docValue(doc,params.v0), "
                 + "params.v1, params.v2),params.v3)",
             scriptTemplate.toString());
         assertEquals("[{v=date}, {v=Z}, {v=YEAR}, {v=null}]", scriptTemplate.params().toString());
@@ -1239,7 +1239,7 @@ public void testTranslateRoundWithTwoParameters() {
         assertNotNull(groupingContext);
         ScriptTemplate scriptTemplate = groupingContext.tail.script();
         assertEquals(
-                "InternalSqlScriptUtils.round(InternalSqlScriptUtils.dateTimeChrono(InternalQlScriptUtils.docValue(doc,params.v0), "
+                "InternalSqlScriptUtils.round(InternalSqlScriptUtils.dateTimeExtract(InternalQlScriptUtils.docValue(doc,params.v0), "
                 + "params.v1, params.v2),params.v3)",
             scriptTemplate.toString());
         assertEquals("[{v=date}, {v=Z}, {v=YEAR}, {v=-2}]", scriptTemplate.params().toString());
@@ -1578,7 +1578,7 @@ public void testOrderByYear() {
         assertThat(eqe.queryContainer().toString().replaceAll("\\s+", ""),
                 endsWith("\"sort\":[{\"_script\":{\"script\":{\"source\":\"InternalQlScriptUtils.nullSafeSortNumeric("
                         +
-                        "InternalSqlScriptUtils.dateTimeChrono(InternalQlScriptUtils.docValue(doc,params.v0)," +
+                        "InternalSqlScriptUtils.dateTimeExtract(InternalQlScriptUtils.docValue(doc,params.v0)," +
                         "params.v1,params.v2))\",\"lang\":\"painless\",\"params\":{\"v0\":\"date\",\"v1\":\"Z\"," +
                         "\"v2\":\"YEAR\"}},\"type\":\"number\",\"order\":\"asc\"}}]}"));
     }
@@ -1726,7 +1726,7 @@ public void testGroupByCastScalar() {
         assertThat(
             ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString()
                 .replaceAll("\\s+", ""),
-            endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" +
+            endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeExtract" +
                         "(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," +
                 "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," +
                 "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}")
@@ -1743,7 +1743,7 @@ public void testGroupByCastScalarWithAlias() {
         assertThat(
             ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString()
                 .replaceAll("\\s+", ""),
-            endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" +
+            endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeExtract" +
                         "(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," +
                 "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," +
                 "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}")
@@ -1760,7 +1760,7 @@ public void testGroupByCastScalarWithNumericRef() {
         assertThat(
             ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString()
                 .replaceAll("\\s+", ""),
-            endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" +
+            endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeExtract" +
                         "(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," +
                 "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," +
                 "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}")
@@ -1779,7 +1779,7 @@ public void testGroupByConvertScalar() {
             assertThat(
                 ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString()
                     .replaceAll("\\s+", ""),
-                endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" +
+                endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeExtract" +
                             "(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," +
                     "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," +
                     "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}")
@@ -1795,7 +1795,7 @@ public void testGroupByConvertScalar() {
             assertThat(
                 ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString()
                     .replaceAll("\\s+", ""),
-                endsWith("{\"source\":\"InternalSqlScriptUtils.dateTimeChrono(" +
+                endsWith("{\"source\":\"InternalSqlScriptUtils.dateTimeExtract(" +
                             "InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)\",\"lang\":\"painless\"," +
                     "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"HOUR_OF_DAY\"}},\"missing_bucket\":true," +
                     "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}")
@@ -1814,7 +1814,7 @@ public void testGroupByConvertScalarWithAlias() {
             assertThat(
                 ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString()
                     .replaceAll("\\s+", ""),
-                endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" +
+                endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeExtract" +
                             "(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," +
                     "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," +
                     "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}")
@@ -1829,7 +1829,7 @@ public void testGroupByConvertScalarWithAlias() {
             assertThat(
                 ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString()
                     .replaceAll("\\s+", ""),
-                endsWith("{\"source\":\"InternalSqlScriptUtils.dateTimeChrono(" +
+                endsWith("{\"source\":\"InternalSqlScriptUtils.dateTimeExtract(" +
                             "InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)\",\"lang\":\"painless\"," +
                     "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"MINUTE_OF_HOUR\"}}," +
                     "\"missing_bucket\":true,\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}")
@@ -1847,7 +1847,7 @@ public void testGroupByConvertScalarWithNumericRef() {
         assertThat(
             ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString()
                 .replaceAll("\\s+", ""),
-            endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" +
+            endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeExtract" +
                         "(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," +
                 "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," +
                 "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}")
@@ -2149,11 +2149,11 @@ public void testChronoFieldBasedDateTimeFunctionsWithMathIntervalAndGroupBy() {
         assertEquals(EsQueryExec.class, p.getClass());
         EsQueryExec eqe = (EsQueryExec) p;
         assertThat(eqe.queryContainer().toString().replaceAll("\\s+", ""), containsString(
-                "{\"terms\":{\"script\":{\"source\":\"InternalSqlScriptUtils.dateTimeChrono("
+                "{\"terms\":{\"script\":{\"source\":\"InternalSqlScriptUtils.dateTimeExtract("
                         + "InternalSqlScriptUtils.add(InternalQlScriptUtils.docValue(doc,params.v0),"
                         + "InternalSqlScriptUtils.intervalYearMonth(params.v1,params.v2)),params.v3,params.v4)\","
                 + "\"lang\":\"painless\",\"params\":{\"v0\":\"date\",\"v1\":\"P1Y\",\"v2\":\"INTERVAL_YEAR\","
-                + "\"v3\":\"Z\",\"v4\":\"" + randomFunction.chronoField().name() + "\"}},\"missing_bucket\":true,"
+                + "\"v3\":\"Z\",\"v4\":\"" + randomFunction.name() + "\"}},\"missing_bucket\":true,"
                 + "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}}"));
     }