Skip to content

Commit 30ae22b

Browse files
authored
ESQL: don't use a Literal for constant_keyword fields when used inside full-text functions (elastic#145632) (elastic#145795)
* ESQL: don't use a Literal for constant_keyword fields when used inside full-text functions (elastic#145632) (cherry picked from commit 5e46b50) * Update for 9.3 specifically * Move the csv file to the right location * Remove all-types index and all its uses since it's interfering with stuff that not backported from "main"
1 parent 6827cbb commit 30ae22b

File tree

3 files changed

+148
-1
lines changed

3 files changed

+148
-1
lines changed

docs/changelog/145632.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
area: ES|QL
2+
issues:
3+
- 145570
4+
pr: 145632
5+
summary: Don't use a Literal for `constant_keyword` fields when used inside full-text
6+
functions
7+
type: bug

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/ReplaceFieldWithConstantOrNull.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
1616
import org.elasticsearch.xpack.esql.core.expression.Literal;
1717
import org.elasticsearch.xpack.esql.core.type.PotentiallyUnmappedKeywordEsField;
18+
import org.elasticsearch.xpack.esql.expression.function.fulltext.FullTextFunction;
1819
import org.elasticsearch.xpack.esql.optimizer.LocalLogicalOptimizerContext;
1920
import org.elasticsearch.xpack.esql.optimizer.rules.RuleUtils;
2021
import org.elasticsearch.xpack.esql.plan.logical.EsRelation;
@@ -113,9 +114,18 @@ private LogicalPlan replaceWithNullOrConstant(
113114
|| plan instanceof OrderBy
114115
|| plan instanceof RegexExtract
115116
|| plan instanceof TopN) {
117+
118+
// full-text functions need actual index fields to construct Lucene queries
119+
// Note: AttributeSet uses semanticEquals for lookups, so any FieldAttribute that refers to the same underlying field as
120+
// one used inside a FullTextFunction is protected here, even if it also appears outside the function (e.g. in a plain equality
121+
// check). This slight loss of constant-folding opportunity is intentional to keep the logic simple.
122+
var fullTextFieldArgsBuilder = AttributeSet.builder();
123+
plan.forEachExpression(FullTextFunction.class, ftf -> ftf.forEachDown(FieldAttribute.class, fullTextFieldArgsBuilder::add));
124+
AttributeSet fullTextFieldArgs = fullTextFieldArgsBuilder.build();
125+
116126
return plan.transformExpressionsOnlyUp(FieldAttribute.class, f -> {
117127
if (attrToConstant.containsKey(f)) {// handle constant values field and use the value itself instead
118-
return attrToConstant.get(f);
128+
return fullTextFieldArgs.contains(f) ? f : attrToConstant.get(f);
119129
} else {// handle missing fields and replace them with null
120130
return shouldBeRetained.test(f) ? f : Literal.of(f, null);
121131
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2126,6 +2126,136 @@ public void testKnnOnMissingField() {
21262126
assertThat(Expressions.name(fullTextFunction.field()), equalTo("text"));
21272127
}
21282128

2129+
public void testFullTextFunctionOnConstantField() {
2130+
String functionName = randomFrom("match", "match_phrase");
2131+
var plan = plan(String.format(Locale.ROOT, """
2132+
from test
2133+
| where %s(first_name, "John")
2134+
""", functionName));
2135+
2136+
var searchStats = new EsqlTestUtils.TestSearchStats() {
2137+
@Override
2138+
public String constantValue(FieldAttribute.FieldName name) {
2139+
if (name.string().equals("first_name")) {
2140+
return "John";
2141+
}
2142+
return null;
2143+
}
2144+
};
2145+
var localPlan = localPlan(plan, searchStats);
2146+
2147+
var limit = as(localPlan, Limit.class);
2148+
var filter = as(limit.child(), Filter.class);
2149+
var fullTextFunction = as(filter.condition(), SingleFieldFullTextFunction.class);
2150+
// The field must remain a FieldAttribute — not replaced with a constant Literal
2151+
assertThat(fullTextFunction.field(), instanceOf(FieldAttribute.class));
2152+
assertThat(Expressions.name(fullTextFunction.field()), equalTo("first_name"));
2153+
}
2154+
2155+
public void testConstantFieldReplacedOutsideFullTextFunction() {
2156+
var plan = plan("""
2157+
from test
2158+
| where match_phrase(first_name, "John") and last_name == "Doe"
2159+
""");
2160+
2161+
var searchStats = new EsqlTestUtils.TestSearchStats() {
2162+
@Override
2163+
public String constantValue(FieldAttribute.FieldName name) {
2164+
if (name.string().equals("last_name")) {
2165+
return "Doe";
2166+
}
2167+
return null;
2168+
}
2169+
};
2170+
var localPlan = localPlan(plan, searchStats);
2171+
2172+
var limit = as(localPlan, Limit.class);
2173+
var filter = as(limit.child(), Filter.class);
2174+
// last_name is a constant field with value "Doe", so last_name == "Doe" folds to true
2175+
// and is eliminated from the AND, leaving only the full-text function in the condition.
2176+
// If constant substitution had NOT happened, the condition would still be an And.
2177+
var matchPhrase = as(filter.condition(), SingleFieldFullTextFunction.class);
2178+
assertThat(matchPhrase.field(), instanceOf(FieldAttribute.class));
2179+
assertThat(Expressions.name(matchPhrase.field()), equalTo("first_name"));
2180+
}
2181+
2182+
public void testMatchOperatorOnConstantField() {
2183+
var plan = plan("""
2184+
from test
2185+
| where first_name : "John"
2186+
""");
2187+
2188+
var searchStats = new EsqlTestUtils.TestSearchStats() {
2189+
@Override
2190+
public String constantValue(FieldAttribute.FieldName name) {
2191+
if (name.string().equals("first_name")) {
2192+
return "John";
2193+
}
2194+
return null;
2195+
}
2196+
};
2197+
var localPlan = localPlan(plan, searchStats);
2198+
2199+
var limit = as(localPlan, Limit.class);
2200+
var filter = as(limit.child(), Filter.class);
2201+
var matchOp = as(filter.condition(), SingleFieldFullTextFunction.class);
2202+
assertThat(matchOp.field(), instanceOf(FieldAttribute.class));
2203+
assertThat(Expressions.name(matchOp.field()), equalTo("first_name"));
2204+
}
2205+
2206+
public void testSameConstantFieldProtectedInFullTextAndOutside() {
2207+
var plan = plan("""
2208+
from test
2209+
| where match(first_name, "John") and first_name == "John"
2210+
""");
2211+
2212+
var searchStats = new EsqlTestUtils.TestSearchStats() {
2213+
@Override
2214+
public String constantValue(FieldAttribute.FieldName name) {
2215+
if (name.string().equals("first_name")) {
2216+
return "John";
2217+
}
2218+
return null;
2219+
}
2220+
};
2221+
var localPlan = localPlan(plan, searchStats);
2222+
2223+
var limit = as(localPlan, Limit.class);
2224+
var filter = as(limit.child(), Filter.class);
2225+
// first_name is protected everywhere because it appears in a full-text function;
2226+
// semantic equality in AttributeSet means all references to the same field are kept.
2227+
filter.condition().forEachDown(FieldAttribute.class, fa -> assertThat(Expressions.name(fa), equalTo("first_name")));
2228+
filter.condition().forEachDown(SingleFieldFullTextFunction.class, ftf -> assertThat(ftf.field(), instanceOf(FieldAttribute.class)));
2229+
}
2230+
2231+
public void testMultipleFullTextFunctionsOnConstantFields() {
2232+
var plan = plan("""
2233+
from test
2234+
| where match(first_name, "John") and match_phrase(last_name, "Doe")
2235+
""");
2236+
2237+
var searchStats = new EsqlTestUtils.TestSearchStats() {
2238+
@Override
2239+
public String constantValue(FieldAttribute.FieldName name) {
2240+
if (name.string().equals("first_name") || name.string().equals("last_name")) {
2241+
return "constant";
2242+
}
2243+
return null;
2244+
}
2245+
};
2246+
var localPlan = localPlan(plan, searchStats);
2247+
2248+
var limit = as(localPlan, Limit.class);
2249+
var filter = as(limit.child(), Filter.class);
2250+
var and = as(filter.condition(), And.class);
2251+
var match = as(and.left(), SingleFieldFullTextFunction.class);
2252+
assertThat(match.field(), instanceOf(FieldAttribute.class));
2253+
assertThat(Expressions.name(match.field()), equalTo("first_name"));
2254+
var matchPhrase = as(and.right(), SingleFieldFullTextFunction.class);
2255+
assertThat(matchPhrase.field(), instanceOf(FieldAttribute.class));
2256+
assertThat(Expressions.name(matchPhrase.field()), equalTo("last_name"));
2257+
}
2258+
21292259
private static PhysicalPlan physicalPlan(LogicalPlan logicalPlan, Analyzer analyzer) {
21302260
var mapper = new Mapper();
21312261
return mapper.map(new Versioned<>(logicalPlan, analyzer.context().minimumVersion()));

0 commit comments

Comments
 (0)