Skip to content

Commit 7694a58

Browse files
committed
Fix IT
Signed-off-by: Lantao Jin <ltjin@amazon.com>
1 parent 7111912 commit 7694a58

8 files changed

Lines changed: 56 additions & 50 deletions

File tree

docs/user/ppl/cmd/mvcombine.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,26 @@ mvcombine <field>
2727

2828
---
2929

30+
## Configuration
31+
32+
The mvcombine command leverages OpenSearch's top hits aggregation pushdown, which requires increasing the `index.max_inner_result_window` setting to 10000 or larger.
33+
34+
Change the `index.max_inner_result_window` to `10000`:
35+
36+
```bash ppl
37+
curl -sS -H 'Content-Type: application/json' \
38+
-X PUT localhost:9200/mvcombine_data/_settings \
39+
-d '{"index" : {"max_inner_result_window" : "10000"}}'
40+
```
41+
42+
```text
43+
{
44+
"acknowledged": true
45+
}
46+
```
47+
48+
---
49+
3050
## Example 1: Basic mvcombine
3151

3252
Given the following input rows:

doctest/markdown_parser.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class MarkdownDocTestParser:
3434

3535
# Regex to match Markdown code fences with optional attributes
3636
CODE_FENCE_PATTERN = re.compile(
37-
r'^```(\w+)([^\n]*?)\s*\n' # ```language [attributes] (no newlines in attributes)
37+
r'^```([\w][\w ]*\w|\w+)([^\n]*?)\s*\n' # ```language [attributes] (language may contain spaces, e.g. "bash ppl")
3838
r'(.*?)' # code content (non-greedy)
3939
r'^```\s*$', # closing ```
4040
re.MULTILINE | re.DOTALL

doctest/test_docs.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,9 +472,9 @@ def create_markdown_suite(filepaths, category_name, setup_func):
472472
transform=transform,
473473
)
474474

475-
# Prepare globs for bash commands
475+
# Prepare globs for bash commands (needed when input_langs includes "bash ppl")
476476
test_globs = {}
477-
if "bash" in category_name:
477+
if "bash" in category_name or "bash ppl" in input_langs:
478478
test_globs = {
479479
"sh": partial(
480480
subprocess.run,

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteMvCombineCommandIT.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.util.List;
1818
import org.json.JSONArray;
1919
import org.json.JSONObject;
20+
import org.junit.After;
2021
import org.junit.jupiter.api.Assertions;
2122
import org.junit.jupiter.api.Test;
2223
import org.opensearch.client.ResponseException;
@@ -31,6 +32,12 @@ public void init() throws Exception {
3132
super.init();
3233
enableCalcite();
3334
loadIndex(Index.MVCOMBINE);
35+
updateIndexSettings(INDEX, "{ \"index\": { \"max_inner_result_window\":" + 10000 + " } }");
36+
}
37+
38+
@After
39+
public void afterTest() throws IOException {
40+
updateIndexSettings(INDEX, "{ \"index\": { \"max_inner_result_window\":" + 100 + " } }");
3441
}
3542

3643
// ---------------------------

integ-test/src/test/java/org/opensearch/sql/ppl/NewAddedCommandsIT.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,12 +246,17 @@ private void verifyQuery(JSONObject result) throws IOException {
246246
public void testMvCombineUnsupportedInV2() throws IOException {
247247
JSONObject result;
248248
try {
249+
updateIndexSettings(
250+
TEST_INDEX_BANK, "{ \"index\": { \"max_inner_result_window\":" + 10000 + " } }");
249251
result =
250252
executeQuery(
251253
String.format(
252254
"source=%s | fields state, city, age | mvcombine age", TEST_INDEX_BANK));
253255
} catch (ResponseException e) {
254256
result = new JSONObject(TestUtils.getResponseBody(e.getResponse()));
257+
} finally {
258+
updateIndexSettings(
259+
TEST_INDEX_BANK, "{ \"index\": { \"max_inner_result_window\":" + 100 + " } }");
255260
}
256261
verifyQuery(result);
257262
}

integ-test/src/test/java/org/opensearch/sql/security/CalciteCrossClusterSearchIT.java

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -389,20 +389,26 @@ public void testCrossClusterAppend() throws IOException {
389389
/** CrossClusterSearchIT Test for mvcombine. */
390390
@Test
391391
public void testCrossClusterMvcombine() throws IOException {
392-
393-
JSONObject result =
394-
executeQuery(
395-
String.format(
396-
"search source=%s | where firstname='Hattie' or firstname='Nanette' "
397-
+ "| fields firstname, age | mvcombine age",
398-
TEST_INDEX_BANK_REMOTE));
399-
400-
verifyColumn(result, columnName("firstname"), columnName("age"));
401-
402-
verifyDataRows(
403-
result,
404-
rows("Hattie", new org.json.JSONArray().put(36)),
405-
rows("Nanette", new org.json.JSONArray().put(28)));
392+
try {
393+
updateIndexSettings(
394+
TEST_INDEX_BANK_REMOTE, "{ \"index\": { \"max_inner_result_window\":" + 10000 + " } }");
395+
JSONObject result =
396+
executeQuery(
397+
String.format(
398+
"search source=%s | where firstname='Hattie' or firstname='Nanette' "
399+
+ "| fields firstname, age | mvcombine age",
400+
TEST_INDEX_BANK_REMOTE));
401+
402+
verifyColumn(result, columnName("firstname"), columnName("age"));
403+
404+
verifyDataRows(
405+
result,
406+
rows("Hattie", new org.json.JSONArray().put(36)),
407+
rows("Nanette", new org.json.JSONArray().put(28)));
408+
} finally {
409+
updateIndexSettings(
410+
TEST_INDEX_BANK_REMOTE, "{ \"index\": { \"max_inner_result_window\":" + 100 + " } }");
411+
}
406412
}
407413

408414
/** CrossClusterSearchIT Test for fieldformat. */

opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchRestClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public boolean updateIndexSettings(Map<String, Object> settings, String... index
129129
return response.isAcknowledged();
130130
} catch (IOException e) {
131131
throw new IllegalStateException(
132-
String.format("Failed to update index settings %s for %s", settings, indexExpression, e));
132+
String.format("Failed to update index settings %s for %s", settings, indexExpression), e);
133133
}
134134
}
135135

opensearch/src/main/java/org/opensearch/sql/opensearch/request/AggregateAnalyzer.java

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -625,38 +625,6 @@ yield switch (functionName) {
625625
};
626626
}
627627

628-
private static TopHitsAggregationBuilder getTopHitsAggregationBuilder(
629-
AggregateCall aggCall,
630-
List<Pair<RexNode, String>> args,
631-
String aggName,
632-
AggregateBuilderHelper helper,
633-
Integer topHitsSize) {
634-
// Disable fetchSource since TopHitsParser only parses fetchField currently.
635-
TopHitsAggregationBuilder topHitsAggregationBuilder =
636-
AggregationBuilders.topHits(aggName).from(0).size(topHitsSize);
637-
List<String> sources = new ArrayList<>();
638-
List<SearchSourceBuilder.ScriptField> scripts = new ArrayList<>();
639-
args.forEach(
640-
rex -> {
641-
if (rex.getKey() instanceof RexInputRef) {
642-
sources.add(helper.inferNamedField(rex.getKey()).getReference());
643-
} else if (rex.getKey() instanceof RexCall || rex.getKey() instanceof RexLiteral) {
644-
scripts.add(
645-
new SearchSourceBuilder.ScriptField(
646-
rex.getValue(), helper.inferScript(rex.getKey()).getScript(), false));
647-
} else {
648-
throw new AggregateAnalyzerException(
649-
String.format(
650-
"Unsupported push-down aggregator %s due to rex type is %s",
651-
aggCall.getAggregation(), rex.getKey().getKind()));
652-
}
653-
});
654-
topHitsAggregationBuilder.fetchSource(
655-
sources.stream().distinct().toArray(String[]::new), new String[0]);
656-
topHitsAggregationBuilder.scriptFields(scripts);
657-
return topHitsAggregationBuilder;
658-
}
659-
660628
private static boolean supportsMaxMinAggregation(ExprType fieldType) {
661629
ExprType coreType =
662630
(fieldType instanceof OpenSearchDataType)

0 commit comments

Comments
 (0)