Skip to content

Commit acd327a

Browse files
authored
[branch-4.0][fix](search) reject Lucene-syntax search on columns without inverted index (#63857)
## Proposed changes Backport #63637 to branch-4.0.
1 parent 072399d commit acd327a

4 files changed

Lines changed: 183 additions & 5 deletions

File tree

fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3801,6 +3801,9 @@ public Index getInvertedIndex(Column column, List<String> subPath) {
38013801
}
38023802

38033803
public Index getInvertedIndex(Column column, List<String> subPath, String analyzer) {
3804+
if (indexes == null) {
3805+
return null;
3806+
}
38043807
List<Index> invertedIndexes = new ArrayList<>();
38053808
for (Index index : indexes.getIndexes()) {
38063809
if (index.getIndexType() == IndexDef.IndexType.INVERTED) {

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/RewriteSearchToSlots.java

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717

1818
package org.apache.doris.nereids.rules.rewrite;
1919

20+
import org.apache.doris.analysis.IndexDef.IndexType;
21+
import org.apache.doris.catalog.Column;
22+
import org.apache.doris.catalog.Index;
23+
import org.apache.doris.catalog.OlapTable;
2024
import org.apache.doris.nereids.exceptions.AnalysisException;
2125
import org.apache.doris.nereids.rules.Rule;
2226
import org.apache.doris.nereids.rules.RuleType;
@@ -127,17 +131,24 @@ private Expression rewriteSearch(Search search, LogicalOlapScan scan) {
127131
"Field '%s' is not VARIANT type for subcolumn access: %s",
128132
parentFieldName, search.getDslString()));
129133
}
134+
String normalizedParentFieldName = parentSlot.getName();
135+
136+
// Check the parent variant column has at least one INVERTED index. The concrete
137+
// subcolumn binding is resolved per-segment in BE, so we only enforce the parent
138+
// level here. See function_search.cpp is_variant_sub branch.
139+
checkInvertedIndexExists(scan.getTable(), normalizedParentFieldName,
140+
search.getDslString(), true);
130141

131142
// Create ElementAt expression for variant subcolumn
132143
// This will be converted to an extracted column slot by VariantSubPathPruning rule
133144
// If the subcolumn doesn't exist, ElementAt will remain and BE will handle it gracefully
134145
childExpr = new ElementAt(parentSlot, new StringLiteral(subcolumnPath));
135-
normalizedFieldName = originalFieldName; // Keep full path for field binding
146+
normalizedFieldName = normalizedParentFieldName + "." + subcolumnPath;
136147

137148
LOG.info(
138149
"Created ElementAt expression for variant subcolumn: parent='{}', "
139150
+ "subcolumn='{}', field_name='{}'",
140-
parentFieldName, subcolumnPath, normalizedFieldName);
151+
normalizedParentFieldName, subcolumnPath, normalizedFieldName);
141152
} else {
142153
// Normal field - find slot directly
143154
Slot slot = findSlotByName(originalFieldName, scan);
@@ -146,6 +157,7 @@ private Expression rewriteSearch(Search search, LogicalOlapScan scan) {
146157
"Field '%s' not found in table for search: %s",
147158
originalFieldName, search.getDslString()));
148159
}
160+
checkInvertedIndexExists(scan.getTable(), slot.getName(), search.getDslString(), false);
149161
childExpr = slot;
150162
normalizedFieldName = slot.getName();
151163
}
@@ -168,6 +180,53 @@ private Expression rewriteSearch(Search search, LogicalOlapScan scan) {
168180
}
169181
}
170182

183+
/**
184+
* Ensure the column referenced by a Lucene-syntax SEARCH predicate has an inverted index.
185+
* Without this check the BE path would silently fall back to an empty bitmap (i.e. all FALSE),
186+
* which is indistinguishable from "no rows matched" to the user. Throw at planning time so the
187+
* behavior is consistent with referencing a non-existent column.
188+
*
189+
* @param table table backing the LogicalOlapScan
190+
* @param columnName column name (parent column name when isVariantParent)
191+
* @param dsl original DSL, used in the error message
192+
* @param isVariantParent true when {@code columnName} is the parent of a variant subcolumn
193+
* access (e.g. {@code msg.body}); for that case any INVERTED index on
194+
* the parent column is accepted because the concrete subcolumn binding
195+
* is resolved per-segment in BE.
196+
*/
197+
private void checkInvertedIndexExists(OlapTable table, String columnName, String dsl,
198+
boolean isVariantParent) {
199+
Column column = table.getColumn(columnName);
200+
if (column == null) {
201+
// Field existence is already validated by findSlotByName; if we reach here the schema
202+
// changed concurrently. Surface a clear error rather than fall through.
203+
throw new AnalysisException(String.format(
204+
"Column '%s' not found in table '%s' for search: %s",
205+
columnName, table.getName(), dsl));
206+
}
207+
208+
if (isVariantParent) {
209+
for (Index index : table.getIndexes()) {
210+
if (index.getIndexType() != IndexType.INVERTED) {
211+
continue;
212+
}
213+
List<String> columns = index.getColumns();
214+
if (columns != null && !columns.isEmpty()
215+
&& columnName.equalsIgnoreCase(columns.get(0))) {
216+
return;
217+
}
218+
}
219+
} else if (table.getInvertedIndex(column, null) != null) {
220+
return;
221+
}
222+
223+
throw new AnalysisException(String.format(
224+
"Field '%s' has no inverted index, cannot be used in search: %s. "
225+
+ "Create an inverted index on the column first "
226+
+ "(ALTER TABLE ... ADD INDEX ... USING INVERTED).",
227+
columnName, dsl));
228+
}
229+
171230
private Slot findSlotByName(String fieldName, LogicalOlapScan scan) {
172231
// Direct match only - variant subcolumns are handled by caller
173232
for (Slot slot : scan.getOutput()) {

fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/RewriteSearchToSlotsTest.java

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,28 @@
1717

1818
package org.apache.doris.nereids.rules.rewrite;
1919

20+
import org.apache.doris.analysis.IndexDef.IndexType;
21+
import org.apache.doris.catalog.AggregateType;
22+
import org.apache.doris.catalog.Column;
23+
import org.apache.doris.catalog.Index;
24+
import org.apache.doris.catalog.KeysType;
25+
import org.apache.doris.catalog.OlapTable;
26+
import org.apache.doris.catalog.PartitionInfo;
27+
import org.apache.doris.catalog.TableIndexes;
28+
import org.apache.doris.catalog.Type;
2029
import org.apache.doris.nereids.exceptions.AnalysisException;
2130
import org.apache.doris.nereids.rules.Rule;
2231
import org.apache.doris.nereids.trees.expressions.Expression;
2332
import org.apache.doris.nereids.trees.expressions.SearchExpression;
2433
import org.apache.doris.nereids.trees.expressions.SlotReference;
34+
import org.apache.doris.nereids.trees.expressions.functions.scalar.ElementAt;
2535
import org.apache.doris.nereids.trees.expressions.functions.scalar.Search;
2636
import org.apache.doris.nereids.trees.expressions.functions.scalar.SearchDslParser;
2737
import org.apache.doris.nereids.trees.expressions.literal.StringLiteral;
2838
import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
2939
import org.apache.doris.nereids.types.StringType;
3040
import org.apache.doris.nereids.util.PlanConstructor;
41+
import org.apache.doris.thrift.TStorageType;
3142

3243
import com.google.common.collect.ImmutableList;
3344
import org.junit.jupiter.api.Assertions;
@@ -229,7 +240,7 @@ public void testSlotReferenceConsistency() {
229240
@Test
230241
public void testRewriteSearchHandlesCaseInsensitiveField() throws Exception {
231242
LogicalOlapScan scan = new LogicalOlapScan(PlanConstructor.getNextRelationId(),
232-
PlanConstructor.student, ImmutableList.of("db"));
243+
buildStudentWithInvertedIndexOnName(100L), ImmutableList.of("db"));
233244
Search searchFunc = new Search(new StringLiteral("NAME:alice"));
234245

235246
Method rewriteMethod = RewriteSearchToSlots.class.getDeclaredMethod(
@@ -250,6 +261,31 @@ public void testRewriteSearchHandlesCaseInsensitiveField() throws Exception {
250261
Assertions.assertEquals("name", normalizedPlan.getRoot().getField());
251262
}
252263

264+
@Test
265+
public void testRewriteSearchHandlesCaseInsensitiveVariantParentField() throws Exception {
266+
LogicalOlapScan scan = new LogicalOlapScan(PlanConstructor.getNextRelationId(),
267+
buildVariantTableWithInvertedIndex(102L), ImmutableList.of("db"));
268+
Search searchFunc = new Search(new StringLiteral("V.foo:bar"));
269+
270+
Method rewriteMethod = RewriteSearchToSlots.class.getDeclaredMethod(
271+
"rewriteSearch", Search.class, LogicalOlapScan.class);
272+
rewriteMethod.setAccessible(true);
273+
274+
Object rewritten = rewriteMethod.invoke(rewriteRule, searchFunc, scan);
275+
Assertions.assertInstanceOf(SearchExpression.class, rewritten);
276+
277+
SearchExpression searchExpression = (SearchExpression) rewritten;
278+
Assertions.assertEquals(1, searchExpression.getSlotChildren().size());
279+
Assertions.assertTrue(searchExpression.getSlotChildren().get(0) instanceof ElementAt);
280+
ElementAt elementAt = (ElementAt) searchExpression.getSlotChildren().get(0);
281+
Assertions.assertTrue(elementAt.child(0) instanceof SlotReference);
282+
Assertions.assertEquals("v", ((SlotReference) elementAt.child(0)).getName());
283+
284+
SearchDslParser.QsPlan normalizedPlan = searchExpression.getQsPlan();
285+
Assertions.assertEquals("v.foo", normalizedPlan.getFieldBindings().get(0).getFieldName());
286+
Assertions.assertEquals("v.foo", normalizedPlan.getRoot().getField());
287+
}
288+
253289
@Test
254290
public void testRewriteSearchThrowsWhenFieldMissing() throws Exception {
255291
LogicalOlapScan scan = new LogicalOlapScan(PlanConstructor.getNextRelationId(),
@@ -266,4 +302,75 @@ public void testRewriteSearchThrowsWhenFieldMissing() throws Exception {
266302
Assertions.assertInstanceOf(AnalysisException.class, thrown.getCause());
267303
Assertions.assertTrue(thrown.getCause().getMessage().contains("unknown_field"));
268304
}
305+
306+
@Test
307+
public void testRewriteSearchThrowsWhenColumnHasNoInvertedIndex() throws Exception {
308+
// PlanConstructor.student has the 'name' column but no inverted index on it. The rewrite
309+
// must surface a clear error instead of letting BE silently return an empty bitmap.
310+
LogicalOlapScan scan = new LogicalOlapScan(PlanConstructor.getNextRelationId(),
311+
PlanConstructor.student, ImmutableList.of("db"));
312+
Search searchFunc = new Search(new StringLiteral("name:alice"));
313+
314+
Method rewriteMethod = RewriteSearchToSlots.class.getDeclaredMethod(
315+
"rewriteSearch", Search.class, LogicalOlapScan.class);
316+
rewriteMethod.setAccessible(true);
317+
318+
InvocationTargetException thrown = Assertions.assertThrows(InvocationTargetException.class,
319+
() -> rewriteMethod.invoke(rewriteRule, searchFunc, scan));
320+
Assertions.assertNotNull(thrown.getCause());
321+
Assertions.assertInstanceOf(AnalysisException.class, thrown.getCause());
322+
Assertions.assertTrue(thrown.getCause().getMessage().contains("inverted index"),
323+
"Error message should mention inverted index, got: " + thrown.getCause().getMessage());
324+
Assertions.assertTrue(thrown.getCause().getMessage().contains("name"));
325+
}
326+
327+
@Test
328+
public void testRewriteSearchSucceedsWhenColumnHasInvertedIndex() throws Exception {
329+
LogicalOlapScan scan = new LogicalOlapScan(PlanConstructor.getNextRelationId(),
330+
buildStudentWithInvertedIndexOnName(101L), ImmutableList.of("db"));
331+
Search searchFunc = new Search(new StringLiteral("name:alice"));
332+
333+
Method rewriteMethod = RewriteSearchToSlots.class.getDeclaredMethod(
334+
"rewriteSearch", Search.class, LogicalOlapScan.class);
335+
rewriteMethod.setAccessible(true);
336+
337+
Object rewritten = rewriteMethod.invoke(rewriteRule, searchFunc, scan);
338+
Assertions.assertInstanceOf(SearchExpression.class, rewritten);
339+
340+
SearchExpression searchExpression = (SearchExpression) rewritten;
341+
Assertions.assertEquals(1, searchExpression.getSlotChildren().size());
342+
Assertions.assertTrue(searchExpression.getSlotChildren().get(0) instanceof SlotReference);
343+
Assertions.assertEquals("name",
344+
((SlotReference) searchExpression.getSlotChildren().get(0)).getName());
345+
}
346+
347+
private static OlapTable buildStudentWithInvertedIndexOnName(long tableId) {
348+
List<Column> columns = ImmutableList.of(
349+
new Column("id", Type.INT, true, AggregateType.NONE, "0", ""),
350+
new Column("gender", Type.INT, false, AggregateType.NONE, "0", ""),
351+
new Column("name", Type.STRING, true, AggregateType.NONE, "", ""),
352+
new Column("age", Type.INT, true, AggregateType.NONE, "", ""));
353+
Index invertedOnName = new Index(1L, "idx_name", ImmutableList.of("name"),
354+
IndexType.INVERTED, null, "");
355+
OlapTable table = new OlapTable(tableId, "student_with_inverted_index", false, columns,
356+
KeysType.PRIMARY_KEYS, new PartitionInfo(), null,
357+
new TableIndexes(ImmutableList.of(invertedOnName)));
358+
table.setIndexMeta(-1, "student_with_inverted_index", table.getFullSchema(),
359+
0, 0, (short) 0, TStorageType.COLUMN, KeysType.PRIMARY_KEYS);
360+
return table;
361+
}
362+
363+
private static OlapTable buildVariantTableWithInvertedIndex(long tableId) {
364+
List<Column> columns = ImmutableList.of(
365+
new Column("id", Type.INT, true, AggregateType.NONE, "0", ""),
366+
new Column("v", Type.VARIANT, false, AggregateType.NONE, "", ""));
367+
Index invertedOnVariant = new Index(2L, "idx_v", ImmutableList.of("v"),
368+
IndexType.INVERTED, null, "");
369+
OlapTable table = new OlapTable(tableId, "variant_with_inverted_index", false, columns,
370+
KeysType.PRIMARY_KEYS, new PartitionInfo(), null,
371+
new TableIndexes(ImmutableList.of(invertedOnVariant)));
372+
table.setIndexMeta(-1, "variant_with_inverted_index", table.getFullSchema(),
373+
0, 0, (short) 0, TStorageType.COLUMN, KeysType.PRIMARY_KEYS);
374+
return table;
375+
}
269376
}

regression-test/suites/search/test_search_function.groovy

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,20 @@ suite("test_search_function", "p0") {
153153
// Test 21: ALL query test
154154
qt_sql "SELECT /*+SET_VAR(enable_common_expr_pushdown=true) */ id, title FROM ${indexTableName} WHERE search('tags:ALL(machine learning)') ORDER BY id"
155155

156-
// Test 22: Search on non-indexed table (will throw exception)
156+
// Test 22: Search on non-indexed table — must now throw at FE planning time.
157+
// After the fix for Jira CIR-20006, RewriteSearchToSlots refuses to rewrite
158+
// a SEARCH predicate against a column that has no inverted index, with an
159+
// AnalysisException that names the column and points at "inverted index".
160+
boolean threw = false
157161
try {
158162
sql """SELECT /*+SET_VAR(enable_common_expr_pushdown=true) */ id, title FROM ${tableName} WHERE search('title:Machine') ORDER BY id"""
159163
} catch (Exception e) {
164+
threw = true
160165
logger.info(e.getMessage())
161-
assertTrue(e.getMessage().contains("SearchExpr should not be executed without inverted index"))
166+
assertTrue(e.getMessage().contains("inverted index"),
167+
"expected error to mention 'inverted index', got: ${e.getMessage()}")
168+
assertTrue(e.getMessage().contains("title"),
169+
"expected error to mention 'title', got: ${e.getMessage()}")
162170
}
171+
assertTrue(threw, "expected AnalysisException for SEARCH on column without inverted index")
163172
}

0 commit comments

Comments
 (0)