Skip to content

Commit dfa41ea

Browse files
authored
Introduce index.query.max_nested_depth (#12286)
Signed-off-by: Mikhail Khludnev <mkhl@apache.org>
1 parent d869ddd commit dfa41ea

File tree

7 files changed

+153
-5
lines changed

7 files changed

+153
-5
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
5959
- [AdmissionControl] Added changes for AdmissionControl Interceptor and AdmissionControlService for RateLimiting ([#9286](https://github.com/opensearch-project/OpenSearch/pull/9286))
6060
- [Admission Control] Integrate CPU AC with ResourceUsageCollector and add CPU AC stats to nodes/stats ([#10887](https://github.com/opensearch-project/OpenSearch/pull/10887))
6161
- Add support for dependencies in plugin descriptor properties with semver range ([#11441](https://github.com/opensearch-project/OpenSearch/pull/11441))
62+
- Introduce query level setting `index.query.max_nested_depth` limiting nested queries ([#3268](https://github.com/opensearch-project/OpenSearch/issues/3268)
6263

6364
### Dependencies
6465
- Bumps jetty version to 9.4.52.v20230823 to fix GMS-2023-1857 ([#9822](https://github.com/opensearch-project/OpenSearch/pull/9822))

server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java

+1
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
149149
IndexSettings.MAX_ADJACENCY_MATRIX_FILTERS_SETTING,
150150
IndexSettings.MAX_ANALYZED_OFFSET_SETTING,
151151
IndexSettings.MAX_TERMS_COUNT_SETTING,
152+
IndexSettings.MAX_NESTED_QUERY_DEPTH_SETTING,
152153
IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING,
153154
IndexSettings.DEFAULT_FIELD_SETTING,
154155
IndexSettings.QUERY_STRING_LENIENT_SETTING,

server/src/main/java/org/opensearch/index/IndexSettings.java

+26
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,17 @@ public static IndexMergePolicy fromString(String text) {
272272
Property.IndexScope
273273
);
274274

275+
/**
276+
* Index setting describing the maximum number of nested scopes in queries.
277+
* The default maximum is 2<sup>31</sup>-1. 1 means once nesting.
278+
*/
279+
public static final Setting<Integer> MAX_NESTED_QUERY_DEPTH_SETTING = Setting.intSetting(
280+
"index.query.max_nested_depth",
281+
Integer.MAX_VALUE,
282+
1,
283+
Property.Dynamic,
284+
Property.IndexScope
285+
);
275286
/**
276287
* Index setting describing for NGramTokenizer and NGramTokenFilter
277288
* the maximum difference between
@@ -765,6 +776,8 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
765776
private volatile TimeValue searchIdleAfter;
766777
private volatile int maxAnalyzedOffset;
767778
private volatile int maxTermsCount;
779+
780+
private volatile int maxNestedQueryDepth;
768781
private volatile String defaultPipeline;
769782
private volatile String requiredPipeline;
770783
private volatile boolean searchThrottled;
@@ -929,6 +942,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
929942
maxSlicesPerPit = scopedSettings.get(MAX_SLICES_PER_PIT);
930943
maxAnalyzedOffset = scopedSettings.get(MAX_ANALYZED_OFFSET_SETTING);
931944
maxTermsCount = scopedSettings.get(MAX_TERMS_COUNT_SETTING);
945+
maxNestedQueryDepth = scopedSettings.get(MAX_NESTED_QUERY_DEPTH_SETTING);
932946
maxRegexLength = scopedSettings.get(MAX_REGEX_LENGTH_SETTING);
933947
this.tieredMergePolicyProvider = new TieredMergePolicyProvider(logger, this);
934948
this.logByteSizeMergePolicyProvider = new LogByteSizeMergePolicyProvider(logger, this);
@@ -1041,6 +1055,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
10411055
scopedSettings.addSettingsUpdateConsumer(MAX_REFRESH_LISTENERS_PER_SHARD, this::setMaxRefreshListeners);
10421056
scopedSettings.addSettingsUpdateConsumer(MAX_ANALYZED_OFFSET_SETTING, this::setHighlightMaxAnalyzedOffset);
10431057
scopedSettings.addSettingsUpdateConsumer(MAX_TERMS_COUNT_SETTING, this::setMaxTermsCount);
1058+
scopedSettings.addSettingsUpdateConsumer(MAX_NESTED_QUERY_DEPTH_SETTING, this::setMaxNestedQueryDepth);
10441059
scopedSettings.addSettingsUpdateConsumer(MAX_SLICES_PER_SCROLL, this::setMaxSlicesPerScroll);
10451060
scopedSettings.addSettingsUpdateConsumer(MAX_SLICES_PER_PIT, this::setMaxSlicesPerPit);
10461061
scopedSettings.addSettingsUpdateConsumer(DEFAULT_FIELD_SETTING, this::setDefaultFields);
@@ -1557,6 +1572,17 @@ private void setMaxTermsCount(int maxTermsCount) {
15571572
this.maxTermsCount = maxTermsCount;
15581573
}
15591574

1575+
/**
1576+
* @return max level of nested queries and documents
1577+
*/
1578+
public int getMaxNestedQueryDepth() {
1579+
return this.maxNestedQueryDepth;
1580+
}
1581+
1582+
private void setMaxNestedQueryDepth(int maxNestedQueryDepth) {
1583+
this.maxNestedQueryDepth = maxNestedQueryDepth;
1584+
}
1585+
15601586
/**
15611587
* Returns the maximum number of allowed script_fields to retrieve in a search request
15621588
*/

server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -322,10 +322,13 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
322322
try {
323323
context.setParentFilter(parentFilter);
324324
context.nestedScope().nextLevel(nestedObjectMapper);
325-
innerQuery = this.query.toQuery(context);
325+
try {
326+
innerQuery = this.query.toQuery(context);
327+
} finally {
328+
context.nestedScope().previousLevel();
329+
}
326330
} finally {
327331
context.setParentFilter(previousParentFilter);
328-
context.nestedScope().previousLevel();
329332
}
330333

331334
// ToParentBlockJoinQuery requires that the inner query only matches documents

server/src/main/java/org/opensearch/index/query/QueryShardContext.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ private QueryShardContext(
256256
this.bitsetFilterCache = bitsetFilterCache;
257257
this.indexFieldDataService = indexFieldDataLookup;
258258
this.allowUnmappedFields = indexSettings.isDefaultAllowUnmappedFields();
259-
this.nestedScope = new NestedScope();
259+
this.nestedScope = new NestedScope(indexSettings);
260260
this.scriptService = scriptService;
261261
this.indexSettings = indexSettings;
262262
this.searcher = searcher;
@@ -270,7 +270,7 @@ private void reset() {
270270
allowUnmappedFields = indexSettings.isDefaultAllowUnmappedFields();
271271
this.lookup = null;
272272
this.namedQueries.clear();
273-
this.nestedScope = new NestedScope();
273+
this.nestedScope = new NestedScope(indexSettings);
274274
}
275275

276276
public IndexAnalyzers getIndexAnalyzers() {

server/src/main/java/org/opensearch/index/query/support/NestedScope.java

+21-1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
package org.opensearch.index.query.support;
3434

3535
import org.opensearch.common.annotation.PublicApi;
36+
import org.opensearch.index.IndexSettings;
3637
import org.opensearch.index.mapper.ObjectMapper;
3738

3839
import java.util.Deque;
@@ -47,6 +48,11 @@
4748
public final class NestedScope {
4849

4950
private final Deque<ObjectMapper> levelStack = new LinkedList<>();
51+
private final IndexSettings indexSettings;
52+
53+
public NestedScope(IndexSettings indexSettings) {
54+
this.indexSettings = indexSettings;
55+
}
5056

5157
/**
5258
* @return For the current nested level returns the object mapper that belongs to that
@@ -60,7 +66,21 @@ public ObjectMapper getObjectMapper() {
6066
*/
6167
public ObjectMapper nextLevel(ObjectMapper level) {
6268
ObjectMapper previous = levelStack.peek();
63-
levelStack.push(level);
69+
if (levelStack.size() < indexSettings.getMaxNestedQueryDepth()) {
70+
levelStack.push(level);
71+
} else {
72+
throw new IllegalArgumentException(
73+
"The depth of Nested Query is ["
74+
+ (levelStack.size() + 1)
75+
+ "] has exceeded "
76+
+ "the allowed maximum of ["
77+
+ indexSettings.getMaxNestedQueryDepth()
78+
+ "]. "
79+
+ "This maximum can be set by changing the ["
80+
+ IndexSettings.MAX_NESTED_QUERY_DEPTH_SETTING.getKey()
81+
+ "] index level setting."
82+
);
83+
}
6484
return previous;
6585
}
6686

server/src/test/java/org/opensearch/index/query/NestedQueryBuilderTests.java

+97
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,16 @@
3434

3535
import com.carrotsearch.randomizedtesting.generators.RandomPicks;
3636

37+
import org.apache.lucene.search.BooleanClause;
38+
import org.apache.lucene.search.BooleanQuery;
39+
import org.apache.lucene.search.MatchAllDocsQuery;
3740
import org.apache.lucene.search.MatchNoDocsQuery;
3841
import org.apache.lucene.search.Query;
3942
import org.apache.lucene.search.join.ScoreMode;
4043
import org.opensearch.OpenSearchException;
4144
import org.opensearch.Version;
4245
import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest;
46+
import org.opensearch.cluster.metadata.IndexMetadata;
4347
import org.opensearch.common.compress.CompressedXContent;
4448
import org.opensearch.common.settings.Settings;
4549
import org.opensearch.index.IndexSettings;
@@ -58,6 +62,7 @@
5862
import java.util.Collections;
5963
import java.util.HashMap;
6064
import java.util.Map;
65+
import java.util.Optional;
6166

6267
import static org.opensearch.index.IndexSettingsTests.newIndexMeta;
6368
import static org.opensearch.index.query.InnerHitBuilderTests.randomNestedInnerHits;
@@ -431,4 +436,96 @@ public void testSetParentFilterInContext() throws Exception {
431436
assertNull(queryShardContext.getParentFilter());
432437
verify(innerQueryBuilder).toQuery(queryShardContext);
433438
}
439+
440+
public void testNestedDepthProhibited() throws Exception {
441+
assertThrows(IllegalArgumentException.class, () -> doWithDepth(0, context -> fail("won't call")));
442+
}
443+
444+
public void testNestedDepthAllowed() throws Exception {
445+
ThrowingConsumer<QueryShardContext> check = (context) -> {
446+
NestedQueryBuilder queryBuilder = new NestedQueryBuilder("nested1", new MatchAllQueryBuilder(), ScoreMode.None);
447+
OpenSearchToParentBlockJoinQuery blockJoinQuery = (OpenSearchToParentBlockJoinQuery) queryBuilder.toQuery(context);
448+
Optional<BooleanClause> childLeg = ((BooleanQuery) blockJoinQuery.getChildQuery()).clauses()
449+
.stream()
450+
.filter(c -> c.getOccur() == BooleanClause.Occur.MUST)
451+
.findFirst();
452+
assertTrue(childLeg.isPresent());
453+
assertEquals(new MatchAllDocsQuery(), childLeg.get().getQuery());
454+
};
455+
check.accept(createShardContext());
456+
doWithDepth(randomIntBetween(1, 20), check);
457+
}
458+
459+
public void testNestedDepthOnceOnly() throws Exception {
460+
doWithDepth(1, this::checkOnceNested);
461+
}
462+
463+
public void testNestedDepthDefault() throws Exception {
464+
assertEquals(Integer.MAX_VALUE, createShardContext().getIndexSettings().getMaxNestedQueryDepth());
465+
}
466+
467+
private void checkOnceNested(QueryShardContext ctx) throws Exception {
468+
{
469+
NestedQueryBuilder depth2 = new NestedQueryBuilder(
470+
"nested1",
471+
new NestedQueryBuilder("nested1", new MatchAllQueryBuilder(), ScoreMode.None),
472+
ScoreMode.None
473+
);
474+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> depth2.toQuery(ctx));
475+
assertEquals(
476+
"The depth of Nested Query is [2] has exceeded the allowed maximum of [1]. This maximum can be set by changing the [index.query.max_nested_depth] index level setting.",
477+
e.getMessage()
478+
);
479+
}
480+
{
481+
QueryBuilder mustBjqMustBjq = new BoolQueryBuilder().must(
482+
new NestedQueryBuilder("nested1", new MatchAllQueryBuilder(), ScoreMode.None)
483+
).must(new NestedQueryBuilder("nested1", new MatchAllQueryBuilder(), ScoreMode.None));
484+
BooleanQuery bool = (BooleanQuery) mustBjqMustBjq.toQuery(ctx);
485+
assertEquals(
486+
"Can parse joins one by one without breaching depth limit",
487+
2,
488+
bool.clauses().stream().filter(c -> c.getQuery() instanceof OpenSearchToParentBlockJoinQuery).count()
489+
);
490+
}
491+
}
492+
493+
public void testUpdateMaxDepthSettings() throws Exception {
494+
doWithDepth(2, (ctx) -> {
495+
assertEquals(ctx.getIndexSettings().getMaxNestedQueryDepth(), 2);
496+
NestedQueryBuilder depth2 = new NestedQueryBuilder(
497+
"nested1",
498+
new NestedQueryBuilder("nested1", new MatchAllQueryBuilder(), ScoreMode.None),
499+
ScoreMode.None
500+
);
501+
Query depth2Query = depth2.toQuery(ctx);
502+
assertTrue(depth2Query instanceof OpenSearchToParentBlockJoinQuery);
503+
});
504+
}
505+
506+
void doWithDepth(int depth, ThrowingConsumer<QueryShardContext> test) throws Exception {
507+
QueryShardContext context = createShardContext();
508+
int defLimit = context.getIndexSettings().getMaxNestedQueryDepth();
509+
assertTrue(defLimit > 0);
510+
Settings updateSettings = Settings.builder()
511+
.put(context.getIndexSettings().getSettings())
512+
.put("index.query.max_nested_depth", depth)
513+
.build();
514+
context.getIndexSettings().updateIndexMetadata(IndexMetadata.builder("index").settings(updateSettings).build());
515+
try {
516+
test.accept(context);
517+
} finally {
518+
context.getIndexSettings()
519+
.updateIndexMetadata(
520+
IndexMetadata.builder("index")
521+
.settings(
522+
Settings.builder()
523+
.put(context.getIndexSettings().getSettings())
524+
.put("index.query.max_nested_depth", defLimit)
525+
.build()
526+
)
527+
.build()
528+
);
529+
}
530+
}
434531
}

0 commit comments

Comments
 (0)