Skip to content

Commit 92acbec

Browse files
ishag4Isha Gupta
andauthored
Add include_metadata request parameter for PPL queries opensearch-project#5235 (opensearch-project#5412)
Co-authored-by: Isha Gupta <igupta24@apple.com> Signed-off-by: Isha Gupta <igupta24@apple.com>
1 parent b53a239 commit 92acbec

20 files changed

Lines changed: 853 additions & 28 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,10 @@ public T visitAllFields(AllFields node, C context) {
359359
}
360360

361361
public T visitAllFieldsExcludeMeta(AllFieldsExcludeMeta node, C context) {
362-
return visitChildren(node, context);
362+
// AllFieldsExcludeMeta is an AllFields, so fall back to visitAllFields by default. Visitors
363+
// that need to tell the two apart override this method; visitors that don't would otherwise
364+
// silently return null here and NPE in their caller.
365+
return visitAllFields(node, context);
363366
}
364367

365368
public T visitNestedAllTupleFields(NestedAllTupleFields node, C context) {

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,19 @@ public class Query extends Statement {
2626
protected final UnresolvedPlan plan;
2727
protected final int fetchSize;
2828
private final QueryType queryType;
29+
30+
/**
31+
* Whether the request asked for metadata fields such as {@code _id}, {@code _index} and {@code
32+
* _score} to be kept in the result.
33+
*/
34+
private final boolean includeMetadata;
35+
2936
private HighlightConfig highlightConfig;
3037

38+
public Query(UnresolvedPlan plan, int fetchSize, QueryType queryType) {
39+
this(plan, fetchSize, queryType, false);
40+
}
41+
3142
@Override
3243
public <R, C> R accept(AbstractNodeVisitor<R, C> visitor, C context) {
3344
return visitor.visitQuery(this, context);

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ public class CalcitePlanContext {
8080
*/
8181
@Getter @Setter private boolean isProjectVisited = false;
8282

83+
/**
84+
* Whether to include metadata fields like _id, _index, _score in the result. When true, metadata
85+
* fields are included in wildcard field selections. When false (default), metadata fields are
86+
* excluded.
87+
*/
88+
@Getter @Setter private boolean includeMetadata = false;
89+
8390
private final Stack<RexCorrelVariable> correlVar = new Stack<>();
8491
private final Stack<List<RexNode>> windowPartitions = new Stack<>();
8592

@@ -165,6 +172,7 @@ private CalcitePlanContext(CalcitePlanContext parent) {
165172
this.rexBuilder = parent.rexBuilder; // Share the same rexBuilder
166173
this.functionProperties = parent.functionProperties;
167174
this.highlightConfig = parent.highlightConfig;
175+
this.includeMetadata = parent.includeMetadata; // Preserve parent's metadata setting
168176
this.rexLambdaRefMap = new HashMap<>(); // New map for lambda variables
169177
this.capturedVariables = new ArrayList<>(); // New list for captured variables
170178
this.inLambdaContext = true; // Mark that we're inside a lambda
@@ -219,6 +227,13 @@ public static CalcitePlanContext create(
219227
return new CalcitePlanContext(config, sysLimit, queryType);
220228
}
221229

230+
public static CalcitePlanContext create(
231+
FrameworkConfig config, SysLimit sysLimit, QueryType queryType, boolean includeMetadata) {
232+
CalcitePlanContext context = new CalcitePlanContext(config, sysLimit, queryType);
233+
context.setIncludeMetadata(includeMetadata);
234+
return context;
235+
}
236+
222237
/**
223238
* Executes {@code action} with the thread-local legacy flag set according to the supplied
224239
* settings.

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,10 +726,18 @@ private static void forceProjectExcept(RelBuilder relBuilder, Iterable<RexNode>
726726
*
727727
* <p>2. There is no other project ever visited in the main query
728728
*
729+
* <p>Unless the request asked for metadata via {@code include_metadata}, in which case only the
730+
* forced exclusion of case 1 still applies.
731+
*
729732
* @param context CalcitePlanContext
730733
* @param excludeByForce whether exclude metadata fields by force
731734
*/
732735
private static void tryToRemoveMetaFields(CalcitePlanContext context, boolean excludeByForce) {
736+
// Join and subquery still strip metadata by force to keep their output schemas unambiguous.
737+
if (context.isIncludeMetadata() && !excludeByForce) {
738+
return;
739+
}
740+
733741
if (excludeByForce || !context.isProjectVisited()) {
734742
List<String> originalFields = context.relBuilder.peek().getRowType().getFieldNames();
735743
List<RexNode> metaFieldsRef =

‎core/src/main/java/org/opensearch/sql/executor/QueryService.java‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,20 @@ public void execute(
152152
QueryType queryType,
153153
HighlightConfig highlightConfig,
154154
ResponseListener<ExecutionEngine.QueryResponse> listener) {
155+
execute(plan, queryType, highlightConfig, false, listener);
156+
}
157+
158+
/** Execute with optional highlight config and include metadata flag. */
159+
public void execute(
160+
UnresolvedPlan plan,
161+
QueryType queryType,
162+
HighlightConfig highlightConfig,
163+
boolean includeMetadata,
164+
ResponseListener<ExecutionEngine.QueryResponse> listener) {
155165
if (shouldUseCalcite(queryType)) {
156-
executeWithCalcite(plan, queryType, highlightConfig, listener);
166+
executeWithCalcite(plan, queryType, highlightConfig, includeMetadata, listener);
157167
} else {
168+
// The V2 engine has no notion of metadata fields, so includeMetadata is ignored there.
158169
executeWithLegacy(plan, queryType, listener, Optional.empty());
159170
}
160171
}
@@ -175,20 +186,22 @@ public void explain(
175186
HighlightConfig highlightConfig,
176187
ResponseListener<ExecutionEngine.ExplainResponse> listener,
177188
ExplainMode mode) {
178-
explain(plan, queryType, highlightConfig, listener, mode, null);
189+
explain(plan, queryType, highlightConfig, false, listener, mode, null);
179190
}
180191

181-
/** Explain with optional highlight config and format. */
192+
/** Explain with optional highlight config, include metadata flag, and format. */
182193
public void explain(
183194
UnresolvedPlan plan,
184195
QueryType queryType,
185196
HighlightConfig highlightConfig,
197+
boolean includeMetadata,
186198
ResponseListener<ExecutionEngine.ExplainResponse> listener,
187199
ExplainMode mode,
188200
Format format) {
189201
if (shouldUseCalcite(queryType)) {
190-
explainWithCalcite(plan, queryType, highlightConfig, listener, mode, format);
202+
explainWithCalcite(plan, queryType, highlightConfig, includeMetadata, listener, mode, format);
191203
} else {
204+
// The V2 engine has no notion of metadata fields, so includeMetadata is ignored there.
192205
explainWithLegacy(plan, queryType, listener, mode, Optional.empty());
193206
}
194207
}
@@ -198,6 +211,15 @@ public void executeWithCalcite(
198211
QueryType queryType,
199212
HighlightConfig highlightConfig,
200213
ResponseListener<ExecutionEngine.QueryResponse> listener) {
214+
executeWithCalcite(plan, queryType, highlightConfig, false, listener);
215+
}
216+
217+
public void executeWithCalcite(
218+
UnresolvedPlan plan,
219+
QueryType queryType,
220+
HighlightConfig highlightConfig,
221+
boolean includeMetadata,
222+
ResponseListener<ExecutionEngine.QueryResponse> listener) {
201223
CalcitePlanContext.run(
202224
() -> {
203225
try {
@@ -209,7 +231,10 @@ public void executeWithCalcite(
209231
() -> {
210232
CalcitePlanContext context =
211233
CalcitePlanContext.create(
212-
buildFrameworkConfig(), SysLimit.fromSettings(settings), queryType);
234+
buildFrameworkConfig(),
235+
SysLimit.fromSettings(settings),
236+
queryType,
237+
includeMetadata);
213238

214239
context.setHighlightConfig(highlightConfig);
215240

@@ -278,13 +303,14 @@ public void explainWithCalcite(
278303
HighlightConfig highlightConfig,
279304
ResponseListener<ExecutionEngine.ExplainResponse> listener,
280305
ExplainMode mode) {
281-
explainWithCalcite(plan, queryType, highlightConfig, listener, mode, null);
306+
explainWithCalcite(plan, queryType, highlightConfig, false, listener, mode, null);
282307
}
283308

284309
public void explainWithCalcite(
285310
UnresolvedPlan plan,
286311
QueryType queryType,
287312
HighlightConfig highlightConfig,
313+
boolean includeMetadata,
288314
ResponseListener<ExecutionEngine.ExplainResponse> listener,
289315
ExplainMode mode,
290316
Format format) {
@@ -296,7 +322,10 @@ public void explainWithCalcite(
296322
() -> {
297323
CalcitePlanContext context =
298324
CalcitePlanContext.create(
299-
buildFrameworkConfig(), SysLimit.fromSettings(settings), queryType);
325+
buildFrameworkConfig(),
326+
SysLimit.fromSettings(settings),
327+
queryType,
328+
includeMetadata);
300329
context.setHighlightConfig(highlightConfig);
301330
context.run(
302331
() -> {

‎core/src/main/java/org/opensearch/sql/executor/execution/QueryPlan.java‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,16 @@ public class QueryPlan extends AbstractPlan {
3333

3434
protected final HighlightConfig highlightConfig;
3535

36+
protected final boolean includeMetadata;
37+
3638
/** Constructor. */
3739
public QueryPlan(
3840
QueryId queryId,
3941
QueryType queryType,
4042
UnresolvedPlan plan,
4143
QueryService queryService,
4244
ResponseListener<ExecutionEngine.QueryResponse> listener) {
43-
this(queryId, queryType, plan, queryService, listener, null);
45+
this(queryId, queryType, plan, queryService, listener, null, false);
4446
}
4547

4648
/** Constructor with highlight config. */
@@ -51,12 +53,25 @@ public QueryPlan(
5153
QueryService queryService,
5254
ResponseListener<ExecutionEngine.QueryResponse> listener,
5355
HighlightConfig highlightConfig) {
56+
this(queryId, queryType, plan, queryService, listener, highlightConfig, false);
57+
}
58+
59+
/** Constructor with highlight config and include metadata flag. */
60+
public QueryPlan(
61+
QueryId queryId,
62+
QueryType queryType,
63+
UnresolvedPlan plan,
64+
QueryService queryService,
65+
ResponseListener<ExecutionEngine.QueryResponse> listener,
66+
HighlightConfig highlightConfig,
67+
boolean includeMetadata) {
5468
super(queryId, queryType);
5569
this.plan = plan;
5670
this.queryService = queryService;
5771
this.listener = listener;
5872
this.pageSize = Optional.empty();
5973
this.highlightConfig = highlightConfig;
74+
this.includeMetadata = includeMetadata;
6075
}
6176

6277
/** Constructor with page size. */
@@ -67,20 +82,38 @@ public QueryPlan(
6782
int pageSize,
6883
QueryService queryService,
6984
ResponseListener<ExecutionEngine.QueryResponse> listener) {
85+
this(queryId, queryType, plan, pageSize, queryService, listener, false);
86+
}
87+
88+
/** Constructor with page size and include metadata flag. */
89+
public QueryPlan(
90+
QueryId queryId,
91+
QueryType queryType,
92+
UnresolvedPlan plan,
93+
int pageSize,
94+
QueryService queryService,
95+
ResponseListener<ExecutionEngine.QueryResponse> listener,
96+
boolean includeMetadata) {
7097
super(queryId, queryType);
7198
this.plan = plan;
7299
this.queryService = queryService;
73100
this.listener = listener;
74101
this.pageSize = Optional.of(pageSize);
75102
this.highlightConfig = null;
103+
this.includeMetadata = includeMetadata;
76104
}
77105

78106
@Override
79107
public void execute() {
80108
if (pageSize.isPresent()) {
81-
queryService.execute(new Paginate(pageSize.get(), plan), getQueryType(), listener);
109+
queryService.execute(
110+
new Paginate(pageSize.get(), plan),
111+
getQueryType(),
112+
highlightConfig,
113+
includeMetadata,
114+
listener);
82115
} else {
83-
queryService.execute(plan, getQueryType(), highlightConfig, listener);
116+
queryService.execute(plan, getQueryType(), highlightConfig, includeMetadata, listener);
84117
}
85118
}
86119

@@ -98,7 +131,8 @@ public void explain(
98131
new NotImplementedException(
99132
"`explain` feature for paginated requests is not implemented yet."));
100133
} else {
101-
queryService.explain(plan, getQueryType(), highlightConfig, listener, mode, format);
134+
queryService.explain(
135+
plan, getQueryType(), highlightConfig, includeMetadata, listener, mode, format);
102136
}
103137
}
104138
}

‎core/src/main/java/org/opensearch/sql/executor/execution/QueryPlanFactory.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ public AbstractPlan visitQuery(
117117
node.getPlan(),
118118
node.getFetchSize(),
119119
queryService,
120-
context.getLeft());
120+
context.getLeft(),
121+
node.isIncludeMetadata());
121122
} else {
122123
// This should be picked up by the legacy engine.
123124
throw new UnsupportedCursorRequestException();
@@ -129,7 +130,8 @@ public AbstractPlan visitQuery(
129130
node.getPlan(),
130131
queryService,
131132
context.getLeft(),
132-
node.getHighlightConfig());
133+
node.getHighlightConfig(),
134+
node.isIncludeMetadata());
133135
}
134136
}
135137

‎core/src/test/java/org/opensearch/sql/analysis/SelectExpressionAnalyzerTest.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.opensearch.sql.analysis;
77

88
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
import static org.junit.jupiter.api.Assertions.assertFalse;
910
import static org.mockito.ArgumentMatchers.any;
1011
import static org.mockito.Mockito.doAnswer;
1112
import static org.mockito.Mockito.mock;
@@ -21,6 +22,8 @@
2122
import org.opensearch.sql.analysis.symbol.Namespace;
2223
import org.opensearch.sql.analysis.symbol.Symbol;
2324
import org.opensearch.sql.ast.dsl.AstDSL;
25+
import org.opensearch.sql.ast.expression.AllFields;
26+
import org.opensearch.sql.ast.expression.AllFieldsExcludeMeta;
2427
import org.opensearch.sql.ast.expression.UnresolvedExpression;
2528
import org.opensearch.sql.expression.DSL;
2629
import org.opensearch.sql.expression.NamedExpression;
@@ -86,6 +89,25 @@ protected void assertAnalyzeEqual(
8689
assertEquals(Arrays.asList(expected), analyze(unresolvedExpression));
8790
}
8891

92+
/**
93+
* {@link AllFieldsExcludeMeta} is an {@link org.opensearch.sql.ast.expression.AllFields}, so the
94+
* V2 select-list analyzer must expand it the same way. It used to fall through to the default
95+
* visitor method, which returns null and made {@link SelectExpressionAnalyzer#analyze} throw
96+
* NullPointerException — surfacing as an HTTP 500 on any query wrapped in an implicit select-all.
97+
*/
98+
@Test
99+
public void all_fields_exclude_meta_expands_like_all_fields() {
100+
SelectExpressionAnalyzer analyzer = new SelectExpressionAnalyzer(expressionAnalyzer);
101+
102+
List<NamedExpression> allFields =
103+
analyzer.analyze(List.of(AllFields.of()), analysisContext, optimizer);
104+
List<NamedExpression> excludeMeta =
105+
analyzer.analyze(List.of(AllFieldsExcludeMeta.of()), analysisContext, optimizer);
106+
107+
assertFalse(allFields.isEmpty());
108+
assertEquals(allFields, excludeMeta);
109+
}
110+
89111
@Test
90112
public void testContextWrapperIsolation() {
91113
// Test that context wrapper properly isolates optimizer instances, each wrapper should have its

‎core/src/test/java/org/opensearch/sql/executor/execution/QueryPlanFactoryTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,14 @@ void init() {
5959

6060
@Test
6161
public void create_from_query_should_success() {
62-
Statement query = new Query(plan, 0, queryType);
62+
Statement query = new Query(plan, 0, queryType, false);
6363
AbstractPlan queryExecution = factory.create(query, queryListener, explainListener);
6464
assertTrue(queryExecution instanceof QueryPlan);
6565
}
6666

6767
@Test
6868
public void create_from_explain_should_success() {
69-
Statement query = new Explain(new Query(plan, 0, queryType), queryType);
69+
Statement query = new Explain(new Query(plan, 0, queryType, false), queryType);
7070
AbstractPlan queryExecution = factory.create(query, queryListener, explainListener);
7171
assertTrue(queryExecution instanceof ExplainPlan);
7272
}
@@ -103,7 +103,7 @@ public void no_consumer_response_channel() {
103103
public void create_query_with_fetch_size_which_can_be_paged() {
104104
when(plan.accept(any(CanPaginateVisitor.class), any())).thenReturn(Boolean.TRUE);
105105
factory = new QueryPlanFactory(queryService);
106-
Statement query = new Query(plan, 10, queryType);
106+
Statement query = new Query(plan, 10, queryType, false);
107107
AbstractPlan queryExecution = factory.create(query, queryListener, explainListener);
108108
assertTrue(queryExecution instanceof QueryPlan);
109109
}
@@ -112,7 +112,7 @@ public void create_query_with_fetch_size_which_can_be_paged() {
112112
public void create_query_with_fetch_size_which_cannot_be_paged() {
113113
when(plan.accept(any(CanPaginateVisitor.class), any())).thenReturn(Boolean.FALSE);
114114
factory = new QueryPlanFactory(queryService);
115-
Statement query = new Query(plan, 10, queryType);
115+
Statement query = new Query(plan, 10, queryType, false);
116116
assertThrows(
117117
UnsupportedCursorRequestException.class,
118118
() -> factory.create(query, queryListener, explainListener));

0 commit comments

Comments
 (0)