github-actions[bot] commented on code in PR #69995:
URL: https://github.com/apache/doris/pull/69995#discussion_r4559901558
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/ExpressionTranslatorTest.java:
##########
@@ -127,4 +141,66 @@ void
testRequiredShortCircuitEvaluationSurvivesTranslation() {
Assertions.assertTrue(thriftNode.isSetShortCircuitEvaluation());
Assertions.assertTrue(thriftNode.isShortCircuitEvaluation());
}
+
+ @Test
+ public void testVariantIndexLookupUsesLogicalKeysForMatchAndSearch() {
+ for (boolean escaped : new boolean[] {false, true}) {
+ VariantType type = new VariantType(ImmutableList.of(), 0, false,
10000, 1,
+ false, 0, 64, false);
+ Column column = new Column("v", type.toCatalogDataType());
+ OlapTable table = Mockito.mock(OlapTable.class);
+
Mockito.when(table.getVariantEscapeDotsInKeys()).thenReturn(escaped);
+ List<String> logical = ImmutableList.of("a.b", "50%");
+ SlotReference seed = new SlotReference("v", StringType.INSTANCE,
true);
+ SlotReference slot = new SlotReference(seed.getExprId(), "v",
StringType.INSTANCE, true,
+ ImmutableList.of(), table, column, table, column, logical);
+ PlanTranslatorContext context = new PlanTranslatorContext();
+ context.addExprIdSlotRefPair(slot.getExprId(), new
SlotRef(Type.STRING, true));
+ Assertions.assertNotNull(ExpressionTranslator.INSTANCE.visitMatch(
+ new MatchAny(slot, new VarcharLiteral("abc")), context));
+ Mockito.verify(table).getInvertedIndex(column, logical, null);
+ SearchExpression search = new SearchExpression("v.a%2Eb:abc",
+ SearchDslParser.parseDsl("v.a%2Eb:abc"),
ImmutableList.of(slot));
+
Assertions.assertNotNull(ExpressionTranslator.INSTANCE.visitSearchExpression(search,
context));
+ Mockito.verify(table).getInvertedIndex(column, logical);
+ }
+ }
+
+ @Test
+ public void testVariantSearchFallbackAndDirectLookupChooseTheSameIndex() {
+ List<List<String>> paths = List.of(List.of("a.b"), List.of("a", "b"),
+ List.of("a%2Eb"), List.of("50%"));
+ List<String> storagePaths = List.of("a%2Eb", "a.b", "a%252Eb",
"50%25");
+ ArrayList<VariantField> fields = new ArrayList<>();
+ List<Index> indexes = new ArrayList<>();
+ for (int i = 0; i < storagePaths.size(); ++i) {
+ fields.add(new VariantField(storagePaths.get(i), Type.STRING, "",
PatternType.MATCH_NAME));
+ indexes.add(new Index(i + 1, "idx_" + i, List.of("v"),
IndexType.INVERTED,
+ Map.of("field_pattern", storagePaths.get(i), "parser",
"english"), ""));
+ }
+ Column column = new Column("v", new
org.apache.doris.catalog.VariantType(fields));
+ OlapTable table = Mockito.spy(new OlapTable());
+ table.setIndexes(indexes);
+ table.setVariantEscapeDotsInKeys(true);
+ for (int i = 0; i < paths.size(); ++i) {
+ PlanTranslatorContext context = new PlanTranslatorContext();
+ TupleDescriptor tuple = context.generateTupleDesc();
+ tuple.setTable(table);
+ SlotReference seed = new SlotReference("v", StringType.INSTANCE,
true);
+ SlotReference directSlot = new SlotReference(seed.getExprId(),
"v", StringType.INSTANCE, true,
+ ImmutableList.of(), table, column, table, column,
paths.get(i));
+ SlotDescriptor descriptor = context.createSlotDesc(tuple,
directSlot);
+ // CTE/projection remapping can lose original metadata while its
translated slot still has it.
+ SlotReference remappedSlot = new SlotReference("v",
StringType.INSTANCE, true);
+ context.addExprIdSlotRefPair(remappedSlot.getExprId(), new
SlotRef(descriptor));
+ for (SlotReference slot : List.of(directSlot, remappedSlot)) {
+ SearchExpression search = new SearchExpression("v.a:abc",
+ SearchDslParser.parseDsl("v.a:abc"), List.of(slot));
+ SearchPredicate predicate = (SearchPredicate)
ExpressionTranslator.INSTANCE
+ .visitSearchExpression(search, context);
+ Assertions.assertSame(indexes.get(i),
predicate.getFieldIndexes().get(0));
+ }
+ Mockito.verify(table, Mockito.times(2)).getInvertedIndex(column,
paths.get(i));
Review Comment:
[P2] Update the lookup count for the storage-path fallback
In `testVariantSearchFallbackAndDirectLookupChooseTheSameIndex`, only
`directSlot` calls the two-argument `getInvertedIndex`. `remappedSlot` has no
`originalTable`, so `visitSearchExpression` takes
`getInvertedIndexFromTranslatedSlot`, which now calls
`getInvertedIndexByStoragePath` directly. This `times(2)` therefore observes
one invocation and fails on the first path, even though both returned indexes
are correct. Verify one logical lookup and the separate storage-path lookup, or
rely on the returned-index assertions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
