Skip to content

Commit 1b07536

Browse files
committed
Peng - drop the AST layer implementation and address the comments.
Signed-off-by: Jialiang Liang <jiallian@amazon.com>
1 parent 87bb284 commit 1b07536

35 files changed

Lines changed: 373 additions & 322 deletions

core/src/main/java/org/opensearch/sql/analysis/Analyzer.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@
7676
import org.opensearch.sql.ast.tree.Flatten;
7777
import org.opensearch.sql.ast.tree.GraphLookup;
7878
import org.opensearch.sql.ast.tree.Head;
79-
import org.opensearch.sql.ast.tree.Highlight;
8079
import org.opensearch.sql.ast.tree.Join;
8180
import org.opensearch.sql.ast.tree.Kmeans;
8281
import org.opensearch.sql.ast.tree.Limit;
@@ -571,11 +570,6 @@ public LogicalPlan visitGraphLookup(GraphLookup node, AnalysisContext context) {
571570
throw getOnlyForCalciteException("graphlookup");
572571
}
573572

574-
@Override
575-
public LogicalPlan visitHighlight(Highlight node, AnalysisContext context) {
576-
throw getOnlyForCalciteException("highlight");
577-
}
578-
579573
/** Build {@link ParseExpression} to context and skip to child nodes. */
580574
@Override
581575
public LogicalPlan visitParse(Parse node, AnalysisContext context) {

core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
import org.opensearch.sql.ast.tree.Flatten;
6565
import org.opensearch.sql.ast.tree.GraphLookup;
6666
import org.opensearch.sql.ast.tree.Head;
67-
import org.opensearch.sql.ast.tree.Highlight;
6867
import org.opensearch.sql.ast.tree.Join;
6968
import org.opensearch.sql.ast.tree.Kmeans;
7069
import org.opensearch.sql.ast.tree.Limit;
@@ -496,8 +495,4 @@ public T visitMvExpand(MvExpand node, C context) {
496495
public T visitGraphLookup(GraphLookup node, C context) {
497496
return visitChildren(node, context);
498497
}
499-
500-
public T visitHighlight(Highlight node, C context) {
501-
return visitChildren(node, context);
502-
}
503498
}

core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@
6060
import org.opensearch.sql.ast.tree.FillNull;
6161
import org.opensearch.sql.ast.tree.Filter;
6262
import org.opensearch.sql.ast.tree.Head;
63-
import org.opensearch.sql.ast.tree.Highlight;
64-
import org.opensearch.sql.ast.tree.HighlightConfig;
6563
import org.opensearch.sql.ast.tree.Limit;
6664
import org.opensearch.sql.ast.tree.MinSpanBin;
6765
import org.opensearch.sql.ast.tree.MvCombine;
@@ -586,14 +584,6 @@ public static Trendline trendline(
586584
return new Trendline(sortField, Arrays.asList(computations)).attach(input);
587585
}
588586

589-
public static Highlight highlight(UnresolvedPlan input, HighlightConfig highlightConfig) {
590-
return new Highlight(highlightConfig).attach(input);
591-
}
592-
593-
public static Highlight highlight(UnresolvedPlan input, List<String> highlightFields) {
594-
return new Highlight(new HighlightConfig(highlightFields)).attach(input);
595-
}
596-
597587
public static AppendPipe appendPipe(UnresolvedPlan input, UnresolvedPlan subquery) {
598588

599589
return new AppendPipe(subquery).attach(input);

core/src/main/java/org/opensearch/sql/ast/statement/Query.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import lombok.Setter;
1212
import lombok.ToString;
1313
import org.opensearch.sql.ast.AbstractNodeVisitor;
14+
import org.opensearch.sql.ast.tree.HighlightConfig;
1415
import org.opensearch.sql.ast.tree.UnresolvedPlan;
1516
import org.opensearch.sql.executor.QueryType;
1617

@@ -25,6 +26,7 @@ public class Query extends Statement {
2526
protected final UnresolvedPlan plan;
2627
protected final int fetchSize;
2728
private final QueryType queryType;
29+
private HighlightConfig highlightConfig;
2830

2931
@Override
3032
public <R, C> R accept(AbstractNodeVisitor<R, C> visitor, C context) {

core/src/main/java/org/opensearch/sql/ast/tree/Highlight.java

Lines changed: 0 additions & 41 deletions
This file was deleted.

core/src/main/java/org/opensearch/sql/ast/tree/HighlightConfig.java

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,44 @@
55

66
package org.opensearch.sql.ast.tree;
77

8+
import java.util.LinkedHashMap;
89
import java.util.List;
10+
import java.util.Map;
911

1012
/**
11-
* Bundles highlight configuration: field names (or wildcards) and optional pre/post tags and
12-
* fragment size. Supports both the simple array format ({@code ["*"]}) and the rich OSD object
13-
* format with {@code pre_tags}, {@code post_tags}, {@code fields}, and {@code fragment_size}.
13+
* Bundles highlight configuration: field names (or wildcards) with per-field options, and optional
14+
* global pre/post tags and fragment size. Supports both the simple array format ({@code ["*"]}) and
15+
* the rich OSD object format with {@code pre_tags}, {@code post_tags}, {@code fields}, and {@code
16+
* fragment_size}.
17+
*
18+
* <p>The {@code fields} map keys are field names or wildcards; the values are per-field option maps
19+
* that are passed through to the OpenSearch highlight builder (e.g. {@code fragment_size}, {@code
20+
* number_of_fragments}, {@code type}).
1421
*/
1522
public record HighlightConfig(
16-
List<String> fields, List<String> preTags, List<String> postTags, Integer fragmentSize) {
23+
Map<String, Map<String, Object>> fields,
24+
List<String> preTags,
25+
List<String> postTags,
26+
Integer fragmentSize) {
1727

1828
/** Convenience constructor for the simple array format (fields only, no tag/size overrides). */
19-
public HighlightConfig(List<String> fields) {
20-
this(fields, null, null, null);
29+
public HighlightConfig(List<String> fieldNames) {
30+
this(toFieldMap(fieldNames), null, null, null);
31+
}
32+
33+
/** Returns the field names as a list (for display and iteration). */
34+
public List<String> fieldNames() {
35+
return fields == null ? List.of() : List.copyOf(fields.keySet());
36+
}
37+
38+
private static Map<String, Map<String, Object>> toFieldMap(List<String> fieldNames) {
39+
if (fieldNames == null) {
40+
return null;
41+
}
42+
Map<String, Map<String, Object>> map = new LinkedHashMap<>();
43+
for (String name : fieldNames) {
44+
map.put(name, Map.of());
45+
}
46+
return map;
2147
}
2248
}

core/src/main/java/org/opensearch/sql/calcite/CalcitePlanContext.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ private CalcitePlanContext(CalcitePlanContext parent) {
9898
this.relBuilder = parent.relBuilder; // Share the same relBuilder
9999
this.rexBuilder = parent.rexBuilder; // Share the same rexBuilder
100100
this.functionProperties = parent.functionProperties;
101+
this.highlightConfig = parent.highlightConfig;
101102
this.rexLambdaRefMap = new HashMap<>(); // New map for lambda variables
102103
this.capturedVariables = new ArrayList<>(); // New list for captured variables
103104
this.inLambdaContext = true; // Mark that we're inside a lambda

core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@
123123
import org.opensearch.sql.ast.tree.GraphLookup;
124124
import org.opensearch.sql.ast.tree.GraphLookup.Direction;
125125
import org.opensearch.sql.ast.tree.Head;
126-
import org.opensearch.sql.ast.tree.Highlight;
127126
import org.opensearch.sql.ast.tree.Join;
128127
import org.opensearch.sql.ast.tree.Kmeans;
129128
import org.opensearch.sql.ast.tree.Lookup;
@@ -157,9 +156,9 @@
157156
import org.opensearch.sql.ast.tree.Values;
158157
import org.opensearch.sql.ast.tree.Window;
159158
import org.opensearch.sql.calcite.plan.AliasFieldsWrappable;
159+
import org.opensearch.sql.calcite.plan.HighlightPushDown;
160160
import org.opensearch.sql.calcite.plan.OpenSearchConstants;
161161
import org.opensearch.sql.calcite.plan.rel.LogicalGraphLookup;
162-
import org.opensearch.sql.calcite.plan.rel.LogicalHighlight;
163162
import org.opensearch.sql.calcite.plan.rel.LogicalSystemLimit;
164163
import org.opensearch.sql.calcite.plan.rel.LogicalSystemLimit.SystemLimitType;
165164
import org.opensearch.sql.calcite.utils.BinUtils;
@@ -229,16 +228,17 @@ public RelNode visitRelation(Relation node, CalcitePlanContext context) {
229228
context.relBuilder.scan(node.getTableQualifiedName().getParts());
230229
RelNode scan = context.relBuilder.peek();
231230

232-
if (scan instanceof AliasFieldsWrappable) {
233-
((AliasFieldsWrappable) scan).wrapProjectForAliasFields(context.relBuilder);
231+
// Eagerly push down highlight config to the scan (highlight is a scan hint, not an operator)
232+
if (context.getHighlightConfig() != null && scan instanceof HighlightPushDown) {
233+
RelNode newScan = ((HighlightPushDown) scan).pushDownHighlight(context.getHighlightConfig());
234+
context.relBuilder.build(); // pop old scan
235+
context.relBuilder.push(newScan);
236+
scan = newScan;
237+
context.setHighlightConfig(null); // consumed
234238
}
235239

236-
// Wrap with LogicalHighlight if highlight config is set on context (from API request)
237-
if (context.getHighlightConfig() != null) {
238-
RelNode input = context.relBuilder.build();
239-
LogicalHighlight highlight = LogicalHighlight.create(input, context.getHighlightConfig());
240-
context.relBuilder.push(highlight);
241-
context.setHighlightConfig(null); // Clear for join safety
240+
if (scan instanceof AliasFieldsWrappable) {
241+
((AliasFieldsWrappable) scan).wrapProjectForAliasFields(context.relBuilder);
242242
}
243243

244244
return context.relBuilder.peek();
@@ -3199,13 +3199,6 @@ static ChartConfig fromArguments(ArgumentMap argMap) {
31993199
}
32003200
}
32013201

3202-
@Override
3203-
public RelNode visitHighlight(Highlight node, CalcitePlanContext context) {
3204-
context.setHighlightConfig(node.getHighlightConfig());
3205-
visitChildren(node, context);
3206-
return context.relBuilder.peek();
3207-
}
3208-
32093202
@Override
32103203
public RelNode visitTrendline(Trendline node, CalcitePlanContext context) {
32113204
visitChildren(node, context);
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.calcite.plan;
7+
8+
import org.apache.calcite.rel.RelNode;
9+
import org.opensearch.sql.ast.tree.HighlightConfig;
10+
11+
/**
12+
* Interface for scan nodes that support highlight pushdown. Highlight is a scan hint (not a
13+
* relational operator), so it is pushed down eagerly during plan construction rather than via an
14+
* optimizer rule.
15+
*/
16+
public interface HighlightPushDown {
17+
RelNode pushDownHighlight(HighlightConfig highlightConfig);
18+
}

core/src/main/java/org/opensearch/sql/calcite/plan/rel/LogicalHighlight.java

Lines changed: 0 additions & 77 deletions
This file was deleted.

0 commit comments

Comments
 (0)