ESQL - date nanos range bug? (elastic#125345)

Fixes elastic#125439

We were incorrectly formatting nanosecond dates when building lucene queries. We had missed this in our testing because none of the CSV tests were running against Lucene. This happened because the date nanos test data includes multivalue fields. Our warning behavior for multivalue fields is inconsistent between queries run in Lucene and queries run in pure ES|QL without pushdown. Our warning tests, however, require that the specified warnings be present in all execution paths. When we first built the date nanos CSV tests, we worked around this by always using an MV function to unpack the multivalue fields. But we forgot that using an MV function prevents the entire query from being pushed down to Lucene, and thus that path wasn't being tested.

In this PR, I've duplicated many of the tests to have a version that doesn't use the MV function, and uses warningRegex instead of warning. The regex version does not fail if the header is absent, so it's safe to use in both modes. Rewriting the tests this way revealed several situations in which this bug can manifest, all of which are fixed in this PR. I cannot be confidant that there aren't more paths that can trigger this bug and aren't covered by these tests, but I haven't found any yet.

I've left some trace level logging that I found helpful while debugging this.

---------

Co-authored-by: elasticsearchmachine <[email protected]>

Author: 2 people

docs/changelog/125345.yaml

@@ -0,0 +1,6 @@
+pr: 125345
+summary: ESQL - date nanos range bug?
+area: ES|QL
+type: bug
+issues:
+ - 125439

x-pack/plugin/esql/qa/testFixtures/src/main/resources/date_nanos.csv-spec

@@ -550,6 +550,10 @@ public enum Cap {
          * support date diff function on date nanos type, and mixed nanos/millis
          */
         DATE_NANOS_DATE_DIFF(),
+        /**
+         * Indicates that https://github.com/elastic/elasticsearch/issues/125439 (incorrect lucene push down for date nanos) is fixed
+         */
+        FIX_DATE_NANOS_LUCENE_PUSHDOWN_BUG(),
         /**
          * DATE_PARSE supports reading timezones
          */

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

@@ -8,6 +8,8 @@
 
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
 import org.elasticsearch.xpack.esql.capabilities.TranslationAware;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.FoldContext;
@@ -31,18 +33,23 @@
 
 import static java.util.Arrays.asList;
 import static org.elasticsearch.xpack.esql.core.expression.Foldables.valueOf;
+import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME;
+import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_NANOS;
 import static org.elasticsearch.xpack.esql.core.type.DataType.IP;
 import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG;
 import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
 import static org.elasticsearch.xpack.esql.core.util.DateUtils.asDateTime;
 import static org.elasticsearch.xpack.esql.core.util.NumericUtils.unsignedLongAsNumber;
+import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.DEFAULT_DATE_NANOS_FORMATTER;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.DEFAULT_DATE_TIME_FORMATTER;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateTimeToString;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.ipToString;
+import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.nanoTimeToString;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.versionToString;
 
 // BETWEEN or range - is a mix of gt(e) AND lt(e)
 public class Range extends ScalarFunction implements TranslationAware.SingleValueTranslationAware {
+    private static final Logger logger = LogManager.getLogger(Range.class);
 
     private final Expression value, lower, upper;
     private final boolean includeLower, includeUpper;
@@ -210,12 +217,19 @@ private RangeQuery translate(TranslatorHandler handler) {
         String format = null;
 
         DataType dataType = value.dataType();
-        if (DataType.isDateTime(dataType) && DataType.isDateTime(lower.dataType()) && DataType.isDateTime(upper.dataType())) {
+        logger.trace("Translating Range into lucene query.  dataType is [{}] upper is [{}]  lower is [{}]", dataType, lower, upper);
+        if (dataType == DataType.DATETIME && lower.dataType() == DATETIME && upper.dataType() == DATETIME) {
             l = dateTimeToString((Long) l);
             u = dateTimeToString((Long) u);
             format = DEFAULT_DATE_TIME_FORMATTER.pattern();
         }
 
+        if (dataType == DATE_NANOS && lower.dataType() == DATE_NANOS && upper.dataType() == DATE_NANOS) {
+            l = nanoTimeToString((Long) l);
+            u = nanoTimeToString((Long) u);
+            format = DEFAULT_DATE_NANOS_FORMATTER.pattern();
+        }
+
         if (dataType == IP) {
             if (l instanceof BytesRef bytesRef) {
                 l = ipToString(bytesRef);

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/Range.java

@@ -13,6 +13,8 @@
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
 import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
 import org.elasticsearch.xpack.esql.capabilities.TranslationAware;
 import org.elasticsearch.xpack.esql.core.QlIllegalArgumentException;
@@ -50,14 +52,17 @@
 
 import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 import static org.elasticsearch.xpack.esql.core.expression.Foldables.valueOf;
+import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME;
+import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_NANOS;
 import static org.elasticsearch.xpack.esql.core.type.DataType.IP;
 import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG;
 import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
 import static org.elasticsearch.xpack.esql.core.util.NumericUtils.unsignedLongAsNumber;
+import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.DEFAULT_DATE_NANOS_FORMATTER;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.DEFAULT_DATE_TIME_FORMATTER;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.HOUR_MINUTE_SECOND;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.commonType;
-import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateTimeToString;
+import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateWithTypeToString;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.ipToString;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.versionToString;
 
@@ -66,6 +71,8 @@ public abstract class EsqlBinaryComparison extends BinaryComparison
         EvaluatorMapper,
         TranslationAware.SingleValueTranslationAware {
 
+    private static final Logger logger = LogManager.getLogger(EsqlBinaryComparison.class);
+
     private final Map<DataType, EsqlArithmeticOperation.BinaryEvaluator> evaluatorMap;
 
     private final BinaryComparisonOperation functionType;
@@ -375,14 +382,29 @@ private Query translate(TranslatorHandler handler) {
         String format = null;
         boolean isDateLiteralComparison = false;
 
+        logger.trace(
+            "Translating binary comparison with right: [{}<{}>], left: [{}<{}>], attribute: [{}<{}>]",
+            right(),
+            right().dataType(),
+            left(),
+            left().dataType(),
+            attribute,
+            attribute.dataType()
+        );
+
         // TODO: This type coersion layer is copied directly from the QL counterpart code. It's probably not necessary or desireable
         // in the ESQL version. We should instead do the type conversions using our casting functions.
         // for a date constant comparison, we need to use a format for the date, to make sure that the format is the same
         // no matter the timezone provided by the user
         if (value instanceof ZonedDateTime || value instanceof OffsetTime) {
             DateFormatter formatter;
             if (value instanceof ZonedDateTime) {
-                formatter = DEFAULT_DATE_TIME_FORMATTER;
+                // NB: we check the data type of right here because value is the RHS value
+                formatter = switch (right().dataType()) {
+                    case DATETIME -> DEFAULT_DATE_TIME_FORMATTER;
+                    case DATE_NANOS -> DEFAULT_DATE_NANOS_FORMATTER;
+                    default -> throw new EsqlIllegalArgumentException("Found date value in non-date type comparison");
+                };
                 // RangeQueryBuilder accepts an Object as its parameter, but it will call .toString() on the ZonedDateTime instance
                 // which can have a slightly different format depending on the ZoneId used to create the ZonedDateTime
                 // Since RangeQueryBuilder can handle date as String as well, we'll format it as String and provide the format as well.
@@ -408,10 +430,14 @@ private Query translate(TranslatorHandler handler) {
         }
 
         ZoneId zoneId = null;
-        if (DataType.isDateTime(attribute.dataType())) {
+        if (attribute.dataType() == DATETIME) {
             zoneId = zoneId();
-            value = dateTimeToString((Long) value);
+            value = dateWithTypeToString((Long) value, right().dataType());
             format = DEFAULT_DATE_TIME_FORMATTER.pattern();
+        } else if (attribute.dataType() == DATE_NANOS) {
+            zoneId = zoneId();
+            value = dateWithTypeToString((Long) value, right().dataType());
+            format = DEFAULT_DATE_NANOS_FORMATTER.pattern();
         }
         if (this instanceof GreaterThan) {
             return new RangeQuery(source(), name, value, false, null, false, format, zoneId);

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EsqlBinaryComparison.java

@@ -15,6 +15,8 @@
 import org.elasticsearch.compute.data.Block;
 import org.elasticsearch.compute.data.Vector;
 import org.elasticsearch.compute.operator.EvalOperator;
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
 import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
 import org.elasticsearch.xpack.esql.capabilities.TranslationAware;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
@@ -115,6 +117,7 @@
  */
 public class In extends EsqlScalarFunction implements TranslationAware.SingleValueTranslationAware {
     public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "In", In::new);
+    private static final Logger logger = LogManager.getLogger(In.class);
 
     private final Expression value;
     private final List<Expression> list;
@@ -468,6 +471,7 @@ public Query asQuery(TranslatorHandler handler) {
     }
 
     private Query translate(TranslatorHandler handler) {
+        logger.trace("Attempting to generate lucene query for IN expression");
         TypedAttribute attribute = LucenePushdownPredicates.checkIsPushableAttribute(value());
 
         Set<Object> terms = new LinkedHashSet<>();
@@ -501,7 +505,7 @@ private Query translate(TranslatorHandler handler) {
     }
 
     private static boolean needsTypeSpecificValueHandling(DataType fieldType) {
-        return DataType.isDateTime(fieldType) || fieldType == IP || fieldType == VERSION || fieldType == UNSIGNED_LONG;
+        return fieldType == DATETIME || fieldType == DATE_NANOS || fieldType == IP || fieldType == VERSION || fieldType == UNSIGNED_LONG;
     }
 
     private static Query or(Source source, Query left, Query right) {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java

@@ -40,6 +40,8 @@
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.index.search.NestedHelper;
+import org.elasticsearch.logging.LogManager;
+import org.elasticsearch.logging.Logger;
 import org.elasticsearch.search.fetch.StoredFieldsSpec;
 import org.elasticsearch.search.internal.AliasFilter;
 import org.elasticsearch.search.lookup.SearchLookup;
@@ -76,6 +78,8 @@
 import static org.elasticsearch.index.mapper.MappedFieldType.FieldExtractPreference.NONE;
 
 public class EsPhysicalOperationProviders extends AbstractPhysicalOperationProviders {
+    private static final Logger logger = LogManager.getLogger(EsPhysicalOperationProviders.class);
+
     /**
      * Context of each shard we're operating against.
      */
@@ -180,6 +184,7 @@ public Function<org.elasticsearch.compute.lucene.ShardContext, Query> querySuppl
     @Override
     public final PhysicalOperation sourcePhysicalOperation(EsQueryExec esQueryExec, LocalExecutionPlannerContext context) {
         final LuceneOperator.Factory luceneFactory;
+        logger.trace("Query Exec is {}", esQueryExec);
 
         List<Sort> sorts = esQueryExec.sorts();
         assert esQueryExec.estimatedRowSize() != null : "estimated row size not initialized";

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java

@@ -557,6 +557,16 @@ public static long dateNanosToLong(String dateNano, DateFormatter formatter) {
         return DateUtils.toLong(parsed);
     }
 
+    public static String dateWithTypeToString(long dateTime, DataType type) {
+        if (type == DATETIME) {
+            return dateTimeToString(dateTime);
+        }
+        if (type == DATE_NANOS) {
+            return nanoTimeToString(dateTime);
+        }
+        throw new IllegalArgumentException("Unsupported data type [" + type + "]");
+    }
+
     public static String dateTimeToString(long dateTime) {
         return DEFAULT_DATE_TIME_FORMATTER.formatMillis(dateTime);
     }