Skip to content

[refactor](variant) normalize nested search predicate field resolution #61548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eldenmoon wants to merge 2 commits into
base: master
Choose a base branch
from
+198 −135
Open

[refactor](variant) normalize nested search predicate field resolution #61548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b2d685
[refactor](variant) normalize nested search predicate field resolution
eldenmoon Mar 18, 2026
160ad90
[fix](fe) Support Lucene NESTED clauses in SearchDslParser
eldenmoon Mar 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions be/src/exec/common/variant_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -988,11 +988,6 @@ Status VariantCompactionUtil::check_path_stats(const std::vector<RowsetSharedPtr
return Status::OK();
}
}
for (const auto& column : output->tablet_schema()->columns()) {
if (!column->is_variant_type()) {
continue;
}
}
std::unordered_map<int32_t, PathToNoneNullValues> original_uid_to_path_stats;
for (const auto& rs : intputs) {
RETURN_IF_ERROR(aggregate_path_to_stats(rs, &original_uid_to_path_stats));
Expand Down
199 changes: 79 additions & 120 deletions fe/fe-core/src/main/java/org/apache/doris/analysis/SearchDslParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,37 @@ private static void validateFieldsList(List<String> fields) {
}
}

private static String buildFieldPath(SearchParser.FieldPathContext ctx) {
if (ctx == null) {
throw new RuntimeException("Invalid field query: missing field path");
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildFieldPath() throws a generic RuntimeException for a missing field path. That ends up being wrapped as an "Unexpected error parsing search DSL" in parseDsl*, which is misleading for user-input syntax issues. Prefer throwing SearchDslSyntaxException here so callers consistently surface an "Invalid search DSL" / syntax error message instead of an internal/unexpected error.

Suggested change
throw new RuntimeException("Invalid field query: missing field path");
throw new SearchDslSyntaxException("Invalid field query: missing field path");

Copilot uses AI. Check for mistakes.
}

StringBuilder fullPath = new StringBuilder();
List<SearchParser.FieldSegmentContext> segments = ctx.fieldSegment();
for (int i = 0; i < segments.size(); i++) {
if (i > 0) {
fullPath.append('.');
}
String segment = segments.get(i).getText();
if (segment.startsWith("\"") && segment.endsWith("\"")) {
segment = segment.substring(1, segment.length() - 1);
}
fullPath.append(segment);
}
return fullPath.toString();
}

private static String normalizeNestedFieldPath(String fieldPath, @Nullable String nestedPath) {
if (nestedPath == null || nestedPath.isEmpty()) {
return fieldPath;
}
if (fieldPath.equals(nestedPath) || fieldPath.startsWith(nestedPath + ".")) {
throw new SearchDslSyntaxException("Fields in NESTED predicates must be relative to nested path: "
+ nestedPath + ", but got: " + fieldPath);
}
return nestedPath + "." + fieldPath;
}

/**
* Collect all field names from an AST node recursively.
* @param node The AST node to collect from
Expand Down Expand Up @@ -472,6 +503,7 @@ public void syntaxError(org.antlr.v4.runtime.Recognizer<?, ?> recognizer,
// Build AST using first field as placeholder for bare queries, with default operator
QsAstBuilder visitor = new QsAstBuilder(fields.get(0), defaultOperator);
QsNode root = visitor.visit(tree);
validateNestedTopLevelOnly(root);

// Apply multi-field expansion based on type
QsNode expandedRoot;
Expand Down Expand Up @@ -563,6 +595,7 @@ public void syntaxError(org.antlr.v4.runtime.Recognizer<?, ?> recognizer,
// Use constructor with override to avoid mutating shared options object (thread-safety)
QsLuceneModeAstBuilder visitor = new QsLuceneModeAstBuilder(effectiveOptions, fields.get(0));
QsNode root = visitor.visit(tree);
validateNestedTopLevelOnly(root);

// In ES query_string, both best_fields and cross_fields use per-clause expansion
// (each clause is independently expanded across fields). The difference is only
Expand Down Expand Up @@ -646,6 +679,8 @@ private static class QsAstBuilder extends SearchParserBaseVisitor<QsNode> implem
private final Set<String> fieldNames = new LinkedHashSet<>();
// Context stack to track current field name during parsing
private String currentFieldName = null;
// Current nested path when visiting NESTED(path, predicates)
private String currentNestedPath = null;
// Default field for bare queries (without field: prefix)
private final String defaultField;
// Default operator for implicit conjunction (space-separated terms): "AND" or "OR"
Expand Down Expand Up @@ -822,6 +857,9 @@ public QsNode visitAtomClause(SearchParser.AtomClauseContext ctx) {

@Override
public QsNode visitBareQuery(SearchParser.BareQueryContext ctx) {
if (currentNestedPath != null && (currentFieldName == null || currentFieldName.isEmpty())) {
throw new SearchDslSyntaxException("Bare queries are not supported inside NESTED predicates");
}
// Use currentFieldName if inside a field group context (set by visitFieldGroupQuery),
// otherwise fall back to the configured defaultField.
String effectiveField = (currentFieldName != null && !currentFieldName.isEmpty())
Expand Down Expand Up @@ -858,60 +896,29 @@ public QsNode visitNestedQuery(SearchParser.NestedQueryContext ctx) {
if (ctx.NESTED_PATH() == null) {
throw new RuntimeException("Invalid NESTED clause: missing path");
}
String nestedPath = ctx.NESTED_PATH().getText();
QsNode innerQuery = visit(ctx.clause());
if (innerQuery == null) {
throw new RuntimeException("Invalid NESTED clause: missing inner query");
if (currentNestedPath != null) {
throw new SearchDslSyntaxException("Nested NESTED() is not supported");
}

validateNestedFieldPaths(innerQuery, nestedPath);

QsNode node = new QsNode(QsClauseType.NESTED, Collections.singletonList(innerQuery));
node.nestedPath = nestedPath;
return node;
}

private void validateNestedFieldPaths(QsNode node, String nestedPath) {
if (node == null) {
return;
}
if (node.type == QsClauseType.NESTED) {
throw new RuntimeException("Nested NESTED() is not supported: " + nestedPath);
}
if (node.field != null && !node.field.startsWith(nestedPath + ".")) {
throw new RuntimeException("Fields in NESTED query must start with nested path: "
+ nestedPath + ", but got: " + node.field);
}
if (node.children != null) {
for (QsNode child : node.children) {
validateNestedFieldPaths(child, nestedPath);
String nestedPath = ctx.NESTED_PATH().getText();
String previousNestedPath = currentNestedPath;
currentNestedPath = nestedPath;
try {
QsNode innerQuery = visit(ctx.clause());
if (innerQuery == null) {
throw new RuntimeException("Invalid NESTED clause: missing inner query");
}

QsNode node = new QsNode(QsClauseType.NESTED, Collections.singletonList(innerQuery));
node.nestedPath = nestedPath;
return node;
} finally {
currentNestedPath = previousNestedPath;
}
}

@Override
public QsNode visitFieldQuery(SearchParser.FieldQueryContext ctx) {
if (ctx.fieldPath() == null) {
throw new RuntimeException("Invalid field query: missing field path");
}

// Build complete field path from segments (support field.subcolumn syntax)
StringBuilder fullPath = new StringBuilder();
List<SearchParser.FieldSegmentContext> segments = ctx.fieldPath().fieldSegment();

for (int i = 0; i < segments.size(); i++) {
if (i > 0) {
fullPath.append('.');
}
String segment = segments.get(i).getText();
// Remove quotes if present
if (segment.startsWith("\"") && segment.endsWith("\"")) {
segment = segment.substring(1, segment.length() - 1);
}
fullPath.append(segment);
}

String fieldPath = fullPath.toString();
String fieldPath = normalizeNestedFieldPath(buildFieldPath(ctx.fieldPath()), currentNestedPath);
fieldNames.add(fieldPath);

// Set current field context before visiting search value
Expand Down Expand Up @@ -941,21 +948,7 @@ public QsNode visitFieldGroupQuery(SearchParser.FieldGroupQueryContext ctx) {
throw new SearchDslSyntaxException("Invalid field group query: missing field path");
}

// Build complete field path from segments (support field.subcolumn syntax)
StringBuilder fullPath = new StringBuilder();
List<SearchParser.FieldSegmentContext> segments = ctx.fieldPath().fieldSegment();
for (int i = 0; i < segments.size(); i++) {
if (i > 0) {
fullPath.append('.');
}
String segment = segments.get(i).getText();
if (segment.startsWith("\"") && segment.endsWith("\"")) {
segment = segment.substring(1, segment.length() - 1);
}
fullPath.append(segment);
}

String fieldPath = fullPath.toString();
String fieldPath = normalizeNestedFieldPath(buildFieldPath(ctx.fieldPath()), currentNestedPath);
fieldNames.add(fieldPath);

// Set field group context so bare terms inside use this field
Expand Down Expand Up @@ -2075,6 +2068,7 @@ private static class QsLuceneModeAstBuilder extends SearchParserBaseVisitor<QsNo
private final Set<String> fieldNames = new LinkedHashSet<>();
private final SearchOptions options;
private String currentFieldName = null;
private String currentNestedPath = null;
// Override for default field - used in multi-field mode to avoid mutating options
private final String overrideDefaultField;
private int nestingLevel = 0;
Expand Down Expand Up @@ -2301,6 +2295,8 @@ private void collectTermsFromNotClause(SearchParser.NotClauseContext ctx, List<T
} finally {
nestingLevel--;
}
} else if (atomCtx.nestedQuery() != null) {
node = visit(atomCtx.nestedQuery());
} else if (atomCtx.fieldGroupQuery() != null) {
// Field group query (e.g., title:(rock OR jazz))
node = visit(atomCtx.fieldGroupQuery());
Expand Down Expand Up @@ -2464,6 +2460,9 @@ public QsNode visitAtomClause(SearchParser.AtomClauseContext ctx) {

@Override
public QsNode visitBareQuery(SearchParser.BareQueryContext ctx) {
if (currentNestedPath != null && (currentFieldName == null || currentFieldName.isEmpty())) {
throw new SearchDslSyntaxException("Bare queries are not supported inside NESTED predicates");
}
// Use currentFieldName if inside a field group context (set by visitFieldGroupQuery),
// otherwise fall back to the effective default field.
String defaultField = getEffectiveDefaultField();
Expand Down Expand Up @@ -2501,55 +2500,29 @@ public QsNode visitNestedQuery(SearchParser.NestedQueryContext ctx) {
if (ctx.NESTED_PATH() == null) {
throw new RuntimeException("Invalid NESTED clause: missing path");
}
String nestedPath = ctx.NESTED_PATH().getText();
QsNode innerQuery = visit(ctx.clause());
if (innerQuery == null) {
throw new RuntimeException("Invalid NESTED clause: missing inner query");
}

validateNestedFieldPaths(innerQuery, nestedPath);

QsNode node = new QsNode(QsClauseType.NESTED, Collections.singletonList(innerQuery));
node.nestedPath = nestedPath;
return node;
}

private void validateNestedFieldPaths(QsNode node, String nestedPath) {
if (node == null) {
return;
}
if (node.type == QsClauseType.NESTED) {
throw new RuntimeException("Nested NESTED() is not supported: " + nestedPath);
if (currentNestedPath != null) {
throw new SearchDslSyntaxException("Nested NESTED() is not supported");
}
if (node.field != null && !node.field.startsWith(nestedPath + ".")) {
throw new RuntimeException("Fields in NESTED query must start with nested path: "
+ nestedPath + ", but got: " + node.field);
}
if (node.children != null) {
for (QsNode child : node.children) {
validateNestedFieldPaths(child, nestedPath);
String nestedPath = ctx.NESTED_PATH().getText();
String previousNestedPath = currentNestedPath;
currentNestedPath = nestedPath;
try {
QsNode innerQuery = visit(ctx.clause());
if (innerQuery == null) {
throw new RuntimeException("Invalid NESTED clause: missing inner query");
}

QsNode node = new QsNode(QsClauseType.NESTED, Collections.singletonList(innerQuery));
node.nestedPath = nestedPath;
return node;
} finally {
currentNestedPath = previousNestedPath;
}
}

@Override
public QsNode visitFieldQuery(SearchParser.FieldQueryContext ctx) {
// Build complete field path
StringBuilder fullPath = new StringBuilder();
List<SearchParser.FieldSegmentContext> segments = ctx.fieldPath().fieldSegment();

for (int i = 0; i < segments.size(); i++) {
if (i > 0) {
fullPath.append('.');
}
String segment = segments.get(i).getText();
if (segment.startsWith("\"") && segment.endsWith("\"")) {
segment = segment.substring(1, segment.length() - 1);
}
fullPath.append(segment);
}

String fieldPath = fullPath.toString();
String fieldPath = normalizeNestedFieldPath(buildFieldPath(ctx.fieldPath()), currentNestedPath);
fieldNames.add(fieldPath);

String previousFieldName = currentFieldName;
Expand All @@ -2571,21 +2544,7 @@ public QsNode visitFieldGroupQuery(SearchParser.FieldGroupQueryContext ctx) {
throw new SearchDslSyntaxException("Invalid field group query: missing field path");
}

// Build complete field path from segments (support field.subcolumn syntax)
StringBuilder fullPath = new StringBuilder();
List<SearchParser.FieldSegmentContext> segments = ctx.fieldPath().fieldSegment();
for (int i = 0; i < segments.size(); i++) {
if (i > 0) {
fullPath.append('.');
}
String segment = segments.get(i).getText();
if (segment.startsWith("\"") && segment.endsWith("\"")) {
segment = segment.substring(1, segment.length() - 1);
}
fullPath.append(segment);
}

String fieldPath = fullPath.toString();
String fieldPath = normalizeNestedFieldPath(buildFieldPath(ctx.fieldPath()), currentNestedPath);
fieldNames.add(fieldPath);

// Set field group context so bare terms inside use this field
Expand Down Expand Up @@ -2724,7 +2683,7 @@ private static void validateNestedTopLevelOnly(QsNode node, boolean isRoot) {
return;
}
if (node.type == QsClauseType.NESTED && !isRoot) {
throw new RuntimeException("NESTED clause must be evaluated at top level");
throw new SearchDslSyntaxException("NESTED clause must be evaluated at top level");
}
if (node.children == null || node.children.isEmpty()) {
return;
Expand Down
29 changes: 29 additions & 0 deletions fe/fe-core/src/test/java/org/apache/doris/analysis/SearchPredicateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,35 @@ public void testBuildThriftParam() {
Assertions.assertEquals("content", param.field_bindings.get(1).field_name);
}

@Test
public void testNestedRelativeFieldsAreNormalizedBeforeThrift() {
String dsl = "NESTED(data.items, msg:hello AND meta.channel:action)";
SearchDslParser.QsPlan plan = SearchDslParser.parseDsl(dsl, "{\"mode\":\"standard\"}");
List<Expr> children = Arrays.asList(createTestSlotRef("data"), createTestSlotRef("data"));

SearchPredicate predicate = new SearchPredicate(dsl, plan, children, true);

TExprNode thriftNode = new TExprNode();
predicate.accept(ExprToThriftVisitor.INSTANCE, thriftNode);

TSearchParam param = thriftNode.search_param;
Assertions.assertNotNull(param);
Assertions.assertEquals("NESTED", param.root.clause_type);
Assertions.assertEquals("data.items", param.root.nested_path);
Assertions.assertEquals(1, param.root.children.size());
Assertions.assertEquals("AND", param.root.children.get(0).clause_type);
Assertions.assertEquals("data.items.msg", param.root.children.get(0).children.get(0).field_name);
Assertions.assertEquals("data.items.meta.channel", param.root.children.get(0).children.get(1).field_name);

Assertions.assertEquals(2, param.field_bindings.size());
Assertions.assertEquals("data.items.msg", param.field_bindings.get(0).field_name);
Assertions.assertEquals("data", param.field_bindings.get(0).parent_field_name);
Assertions.assertEquals("items.msg", param.field_bindings.get(0).subcolumn_path);
Assertions.assertEquals("data.items.meta.channel", param.field_bindings.get(1).field_name);
Assertions.assertEquals("data", param.field_bindings.get(1).parent_field_name);
Assertions.assertEquals("items.meta.channel", param.field_bindings.get(1).subcolumn_path);
Comment on lines +177 to +182
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test asserts specific ordering of param.field_bindings (indexes 0/1). Field bindings are derived from AST traversal/set insertion order, so the exact order can change with harmless refactors and make the test flaky. Consider asserting the bindings by field_name (e.g., lookup by name or assert the set of names/parent_field_name/subcolumn_path) rather than relying on list position.

Suggested change
Assertions.assertEquals("data.items.msg", param.field_bindings.get(0).field_name);
Assertions.assertEquals("data", param.field_bindings.get(0).parent_field_name);
Assertions.assertEquals("items.msg", param.field_bindings.get(0).subcolumn_path);
Assertions.assertEquals("data.items.meta.channel", param.field_bindings.get(1).field_name);
Assertions.assertEquals("data", param.field_bindings.get(1).parent_field_name);
Assertions.assertEquals("items.meta.channel", param.field_bindings.get(1).subcolumn_path);
Map<String, TSearchFieldBinding> bindingsByFieldName = new HashMap<>();
for (TSearchFieldBinding binding : param.field_bindings) {
bindingsByFieldName.put(binding.field_name, binding);
}
TSearchFieldBinding msgBinding = bindingsByFieldName.get("data.items.msg");
Assertions.assertNotNull(msgBinding);
Assertions.assertEquals("data", msgBinding.parent_field_name);
Assertions.assertEquals("items.msg", msgBinding.subcolumn_path);
TSearchFieldBinding metaChannelBinding = bindingsByFieldName.get("data.items.meta.channel");
Assertions.assertNotNull(metaChannelBinding);
Assertions.assertEquals("data", metaChannelBinding.parent_field_name);
Assertions.assertEquals("items.meta.channel", metaChannelBinding.subcolumn_path);

Copilot uses AI. Check for mistakes.
}

@Test
public void testClone() {
String dsl = "title:hello";
Expand Down
Loading
Loading