From 33d077b04794ebe5fe5db552bfa9b8059159a9d0 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Sun, 18 May 2025 22:01:16 -0700 Subject: [PATCH 1/6] flaky test repo Signed-off-by: Anthony Leong --- .../startree/MetricAggregatorTests.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java index dd53067db05e7..666bf9fc2f6ef 100644 --- a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java +++ b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java @@ -137,7 +137,7 @@ protected Codec getCodec( return new Composite101Codec(Lucene101Codec.Mode.BEST_SPEED, mapperService, testLogger); } - @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/18110") + //@AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/18110") public void testStarTreeDocValues() throws IOException { final List> MAX_LEAF_DOC_VARIATIONS = List.of( () -> 1, @@ -555,10 +555,20 @@ public QueryBuilder getTermsQueryBuilder() { } public QueryBuilder getRangeQueryBuilder() { - return new RangeQueryBuilder(fieldName).from(valueSupplier.get()) + RangeQueryBuilder qb = new RangeQueryBuilder(fieldName).from(valueSupplier.get()) .to(valueSupplier.get()) .includeLower(randomBoolean()) .includeUpper(randomBoolean()); + // Keyword terms are not always supported for range queries. + if (fieldType.equals(DimensionTypes.KEYWORD.name().toLowerCase(Locale.ROOT))) { + // This is essentially a match none query because the value of the keyword will + // never have the string value of "-1". Just using a match none query is not possible + // because the match none query does not correspond with a fieldname, making star tree + // precomputation unavailable in boolean queries as the queries must be on the same + // dimension. + return new TermQueryBuilder(fieldName, String.valueOf(-1)); + } + return qb; } public QueryBuilder getBoolQueryBuilder() { @@ -574,7 +584,7 @@ public QueryBuilder getBoolQueryBuilder() { // SHOULD only on same dimension BoolQueryBuilder shouldOnly = new BoolQueryBuilder().should(new TermQueryBuilder(fieldName, valueSupplier.get())) - .should(new RangeQueryBuilder(fieldName).from(valueSupplier.get()).to(valueSupplier.get())); + .should(getRangeQueryBuilder()); return randomFrom(mustOnly, mustWithShould, shouldOnly); } From 501fbcb19916ba1e1e5daf3343b4873a8335c3f8 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Mon, 19 May 2025 00:25:40 -0700 Subject: [PATCH 2/6] spotless Signed-off-by: Anthony Leong --- .../search/aggregations/startree/MetricAggregatorTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java index 666bf9fc2f6ef..6f392823e8e1c 100644 --- a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java +++ b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java @@ -137,7 +137,7 @@ protected Codec getCodec( return new Composite101Codec(Lucene101Codec.Mode.BEST_SPEED, mapperService, testLogger); } - //@AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/18110") + // @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/18110") public void testStarTreeDocValues() throws IOException { final List> MAX_LEAF_DOC_VARIATIONS = List.of( () -> 1, @@ -559,12 +559,12 @@ public QueryBuilder getRangeQueryBuilder() { .to(valueSupplier.get()) .includeLower(randomBoolean()) .includeUpper(randomBoolean()); - // Keyword terms are not always supported for range queries. + // Keyword terms are not always supported for range queries. if (fieldType.equals(DimensionTypes.KEYWORD.name().toLowerCase(Locale.ROOT))) { // This is essentially a match none query because the value of the keyword will // never have the string value of "-1". Just using a match none query is not possible // because the match none query does not correspond with a fieldname, making star tree - // precomputation unavailable in boolean queries as the queries must be on the same + // precomputation unavailable in boolean queries as the queries must be on the same // dimension. return new TermQueryBuilder(fieldName, String.valueOf(-1)); } From d8d7a580a3e3b130a142fb428b5a585379bf34e6 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Tue, 20 May 2025 13:38:23 -0700 Subject: [PATCH 3/6] cleaned up code with methods, changed how keyword terms are handles in range queries Signed-off-by: Anthony Leong --- .../startree/MetricAggregatorTests.java | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java index 6f392823e8e1c..de450c00a8b1e 100644 --- a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java +++ b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java @@ -137,7 +137,6 @@ protected Codec getCodec( return new Composite101Codec(Lucene101Codec.Mode.BEST_SPEED, mapperService, testLogger); } - // @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/18110") public void testStarTreeDocValues() throws IOException { final List> MAX_LEAF_DOC_VARIATIONS = List.of( () -> 1, @@ -559,32 +558,32 @@ public QueryBuilder getRangeQueryBuilder() { .to(valueSupplier.get()) .includeLower(randomBoolean()) .includeUpper(randomBoolean()); - // Keyword terms are not always supported for range queries. - if (fieldType.equals(DimensionTypes.KEYWORD.name().toLowerCase(Locale.ROOT))) { - // This is essentially a match none query because the value of the keyword will - // never have the string value of "-1". Just using a match none query is not possible - // because the match none query does not correspond with a fieldname, making star tree - // precomputation unavailable in boolean queries as the queries must be on the same - // dimension. - return new TermQueryBuilder(fieldName, String.valueOf(-1)); - } return qb; } public QueryBuilder getBoolQueryBuilder() { // MUST only - BoolQueryBuilder mustOnly = new BoolQueryBuilder().must(getTermQueryBuilder()).must(getRangeQueryBuilder()); + BoolQueryBuilder mustOnly = new BoolQueryBuilder().must(getTermQueryBuilder()); + // Keyword terms are not always supported for range queries. + if (fieldType.equals(DimensionTypes.KEYWORD.name().toLowerCase(Locale.ROOT))) { + mustOnly.must(getTermQueryBuilder()); + } else { + mustOnly.must(getRangeQueryBuilder()); + } // MUST with nested SHOULD on same dimension BoolQueryBuilder mustWithShould = new BoolQueryBuilder().must(getTermQueryBuilder()) - .must( - new BoolQueryBuilder().should(new TermQueryBuilder(fieldName, valueSupplier.get())) - .should(new TermQueryBuilder(fieldName, valueSupplier.get())) - ); + .must(new BoolQueryBuilder().should(getTermQueryBuilder()).should(getTermQueryBuilder())); // SHOULD only on same dimension - BoolQueryBuilder shouldOnly = new BoolQueryBuilder().should(new TermQueryBuilder(fieldName, valueSupplier.get())) - .should(getRangeQueryBuilder()); + BoolQueryBuilder shouldOnly = new BoolQueryBuilder().should(getTermQueryBuilder()); + + // Keyword terms are not always supported for range queries. + if (fieldType.equals(DimensionTypes.KEYWORD.name().toLowerCase(Locale.ROOT))) { + shouldOnly.should(getTermQueryBuilder()); + } else { + shouldOnly.should(getRangeQueryBuilder()); + } return randomFrom(mustOnly, mustWithShould, shouldOnly); } From 148772b77a4700ddb50aa512ada33022663ff3ce Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Tue, 20 May 2025 14:12:56 -0700 Subject: [PATCH 4/6] small changes Signed-off-by: Anthony Leong --- .../search/aggregations/startree/MetricAggregatorTests.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java index de450c00a8b1e..f6bd2018f317d 100644 --- a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java +++ b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java @@ -554,17 +554,15 @@ public QueryBuilder getTermsQueryBuilder() { } public QueryBuilder getRangeQueryBuilder() { - RangeQueryBuilder qb = new RangeQueryBuilder(fieldName).from(valueSupplier.get()) + return new RangeQueryBuilder(fieldName).from(valueSupplier.get()) .to(valueSupplier.get()) .includeLower(randomBoolean()) .includeUpper(randomBoolean()); - return qb; } public QueryBuilder getBoolQueryBuilder() { // MUST only BoolQueryBuilder mustOnly = new BoolQueryBuilder().must(getTermQueryBuilder()); - // Keyword terms are not always supported for range queries. if (fieldType.equals(DimensionTypes.KEYWORD.name().toLowerCase(Locale.ROOT))) { mustOnly.must(getTermQueryBuilder()); } else { @@ -578,7 +576,6 @@ public QueryBuilder getBoolQueryBuilder() { // SHOULD only on same dimension BoolQueryBuilder shouldOnly = new BoolQueryBuilder().should(getTermQueryBuilder()); - // Keyword terms are not always supported for range queries. if (fieldType.equals(DimensionTypes.KEYWORD.name().toLowerCase(Locale.ROOT))) { shouldOnly.should(getTermQueryBuilder()); } else { From e73ce8c4b6b1f33c9f1e244026a70c9b584d7f99 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Tue, 20 May 2025 14:16:37 -0700 Subject: [PATCH 5/6] nonconsequential change Signed-off-by: Anthony Leong --- .../search/aggregations/startree/MetricAggregatorTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java index f6bd2018f317d..c7d5c876a357e 100644 --- a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java +++ b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java @@ -559,6 +559,7 @@ public QueryBuilder getRangeQueryBuilder() { .includeLower(randomBoolean()) .includeUpper(randomBoolean()); } + //HI public QueryBuilder getBoolQueryBuilder() { // MUST only From eb7f003fc537a4bac0fa8dae8a717f61bde1b774 Mon Sep 17 00:00:00 2001 From: Anthony Leong Date: Tue, 20 May 2025 14:18:03 -0700 Subject: [PATCH 6/6] revert recent change Signed-off-by: Anthony Leong --- .../search/aggregations/startree/MetricAggregatorTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java index c7d5c876a357e..f6bd2018f317d 100644 --- a/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java +++ b/server/src/test/java/org/opensearch/search/aggregations/startree/MetricAggregatorTests.java @@ -559,7 +559,6 @@ public QueryBuilder getRangeQueryBuilder() { .includeLower(randomBoolean()) .includeUpper(randomBoolean()); } - //HI public QueryBuilder getBoolQueryBuilder() { // MUST only