From 16dab2bf6dbd20c900067da7ec68eb9e9ccbc50a Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Wed, 30 Oct 2024 14:36:31 +0100 Subject: [PATCH] Also consider isAggregatable for non-indexed fields like ScriptField --- .../rules/physical/local/LucenePushDownUtils.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/LucenePushDownUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/LucenePushDownUtils.java index 3af2aa5c6beb4..625e9ba887db0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/LucenePushDownUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/LucenePushDownUtils.java @@ -19,12 +19,20 @@ public static boolean hasIdenticalDelegate(FieldAttribute attr, SearchStats stat return stats.hasIdenticalDelegate(attr.name()); } + /** + * We see fields as pushable if either they are aggregatable or they are indexed. + * This covers non-indexed cases like AbstractScriptFieldType which hard-coded isAggregatable to true, + * as well as normal FieldAttribute's which can only be pushed down if they are indexed. + * The reason we don't just rely entirely on isAggregatable is because this is often false for normal fields, and could + * also differ from node to node, and we can physically plan each node separately, allowing Lucene pushdown on the nodes that + * support it, and relying on the compute engine for the nodes that do not. + */ public static boolean isPushableFieldAttribute( Expression exp, Predicate hasIdenticalDelegate, Predicate isIndexed ) { - if (exp instanceof FieldAttribute fa && fa.getExactInfo().hasExact() && isIndexed.test(fa)) { + if (exp instanceof FieldAttribute fa && fa.getExactInfo().hasExact() && (fa.field().isAggregatable() || isIndexed.test(fa))) { return (fa.dataType() != DataType.TEXT && fa.dataType() != DataType.SEMANTIC_TEXT) || hasIdenticalDelegate.test(fa); } return false;