Skip to content

Commit 585a04c

Browse files
qianheng-awsqianheng-aws
authored
Enhance the cost computing mechanism and push down context (opensearch-project#4353)
* Make PushDownContext support caching the built request builder Signed-off-by: Heng Qian <qianheng@amazon.com> * Move PushDownContext outside of the AbstractCalciteIndexScan Signed-off-by: Heng Qian <qianheng@amazon.com> * Refactor cost computing Signed-off-by: Heng Qian <qianheng@amazon.com> * migrate explain IT from json format to yaml format Signed-off-by: Heng Qian <qianheng@amazon.com> * Fix opensearch request reuse by equivalent operator Signed-off-by: Heng Qian <qianheng@amazon.com> * fix IT after merging main Signed-off-by: Heng Qian <qianheng@amazon.com> * remove plans assert in execution IT Signed-off-by: Heng Qian <qianheng@amazon.com> * Merging main Signed-off-by: Heng Qian <qianheng@amazon.com> * Add UT for cost evaluation of AbstractCalciteIndexScan Signed-off-by: Heng Qian <qianheng@amazon.com> * merging main Signed-off-by: Heng Qian <qianheng@amazon.com> * merging main Signed-off-by: Heng Qian <qianheng@amazon.com> * merging main Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com>
1 parent c203bda commit 585a04c

171 files changed

Lines changed: 2342 additions & 1232 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import com.fasterxml.jackson.core.JsonProcessingException;
99
import com.fasterxml.jackson.databind.ObjectMapper;
10-
import com.fasterxml.jackson.databind.SerializationFeature;
1110
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
1211
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator;
1312

@@ -27,7 +26,6 @@ public class YamlFormatter {
2726
YAMLGenerator.Feature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS); // Quote numeric strings
2827
yamlFactory.enable(YAMLGenerator.Feature.INDENT_ARRAYS_WITH_INDICATOR);
2928
YAML_MAPPER = new ObjectMapper(yamlFactory);
30-
YAML_MAPPER.enable(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS);
3129
}
3230

3331
/**

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

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -270,19 +270,19 @@ public void testExplainWithTimechartAvg() throws IOException {
270270
var result = explainQueryToString("source=events | timechart span=1m avg(cpu_usage) by host");
271271
String expected =
272272
!isPushdownDisabled()
273-
? loadFromFile("expectedOutput/calcite/explain_timechart.json")
274-
: loadFromFile("expectedOutput/calcite/explain_timechart_no_pushdown.json");
275-
assertJsonEqualsIgnoreId(expected, result);
273+
? loadFromFile("expectedOutput/calcite/explain_timechart.yaml")
274+
: loadFromFile("expectedOutput/calcite/explain_timechart_no_pushdown.yaml");
275+
assertYamlEqualsJsonIgnoreId(expected, result);
276276
}
277277

278278
@Test
279279
public void testExplainWithTimechartCount() throws IOException {
280280
var result = explainQueryToString("source=events | timechart span=1m count() by host");
281281
String expected =
282282
!isPushdownDisabled()
283-
? loadFromFile("expectedOutput/calcite/explain_timechart_count.json")
284-
: loadFromFile("expectedOutput/calcite/explain_timechart_count_no_pushdown.json");
285-
assertJsonEqualsIgnoreId(expected, result);
283+
? loadFromFile("expectedOutput/calcite/explain_timechart_count.yaml")
284+
: loadFromFile("expectedOutput/calcite/explain_timechart_count_no_pushdown.yaml");
285+
assertYamlEqualsJsonIgnoreId(expected, result);
286286
}
287287

288288
@Test
@@ -335,8 +335,8 @@ public void testExplainStatsWithBinsOnTimeField() throws IOException {
335335

336336
@Test
337337
public void testExplainBinWithSpan() throws IOException {
338-
String expected = loadExpectedPlan("explain_bin_span.json");
339-
assertJsonEqualsIgnoreId(
338+
String expected = loadExpectedPlan("explain_bin_span.yaml");
339+
assertYamlEqualsJsonIgnoreId(
340340
expected,
341341
explainQueryToString(
342342
"source=opensearch-sql_test_index_account | bin age span=10 | head 5"));
@@ -362,8 +362,8 @@ public void testExplainBinWithStartEnd() throws IOException {
362362

363363
@Test
364364
public void testExplainBinWithAligntime() throws IOException {
365-
String expected = loadExpectedPlan("explain_bin_aligntime.json");
366-
assertJsonEqualsIgnoreId(
365+
String expected = loadExpectedPlan("explain_bin_aligntime.yaml");
366+
assertYamlEqualsJsonIgnoreId(
367367
expected,
368368
explainQueryToString(
369369
"source=opensearch-sql_test_index_time_data | bin @timestamp span=2h aligntime=latest |"
@@ -413,8 +413,8 @@ public void testEventstatsDistinctCountFunctionExplain() throws IOException {
413413
// Only for Calcite, as v2 gets unstable serialized string for function
414414
@Test
415415
public void testExplainOnAggregationWithSumEnhancement() throws IOException {
416-
String expected = loadExpectedPlan("explain_agg_with_sum_enhancement.json");
417-
assertJsonEqualsIgnoreId(
416+
String expected = loadExpectedPlan("explain_agg_with_sum_enhancement.yaml");
417+
assertYamlEqualsJsonIgnoreId(
418418
expected,
419419
explainQueryToString(
420420
String.format(
@@ -545,16 +545,16 @@ public void testRegexExplain() throws IOException {
545545
String query =
546546
"source=opensearch-sql_test_index_account | regex lastname='^[A-Z][a-z]+$' | head 5";
547547
var result = explainQueryToString(query);
548-
String expected = loadExpectedPlan("explain_regex.json");
549-
assertJsonEqualsIgnoreId(expected, result);
548+
String expected = loadExpectedPlan("explain_regex.yaml");
549+
assertYamlEqualsJsonIgnoreId(expected, result);
550550
}
551551

552552
@Test
553553
public void testRegexNegatedExplain() throws IOException {
554554
String query = "source=opensearch-sql_test_index_account | regex lastname!='.*son$' | head 5";
555555
var result = explainQueryToString(query);
556-
String expected = loadExpectedPlan("explain_regex_negated.json");
557-
assertJsonEqualsIgnoreId(expected, result);
556+
String expected = loadExpectedPlan("explain_regex_negated.yaml");
557+
assertYamlEqualsJsonIgnoreId(expected, result);
558558
}
559559

560560
@Test
@@ -581,8 +581,8 @@ public void testRexExplain() throws IOException {
581581
"source=opensearch-sql_test_index_account | rex field=lastname \\\"(?<initial>^[A-Z])\\\" |"
582582
+ " head 5";
583583
var result = explainQueryToString(query);
584-
String expected = loadExpectedPlan("explain_rex.json");
585-
assertJsonEqualsIgnoreId(expected, result);
584+
String expected = loadExpectedPlan("explain_rex.yaml");
585+
assertYamlEqualsJsonIgnoreId(expected, result);
586586
}
587587

588588
@Test
@@ -628,8 +628,8 @@ public void testPushdownLimitIntoAggregation() throws IOException {
628628
expected,
629629
explainQueryToString("source=opensearch-sql_test_index_account | stats count() by state"));
630630

631-
expected = loadExpectedPlan("explain_limit_agg_pushdown2.json");
632-
assertJsonEqualsIgnoreId(
631+
expected = loadExpectedPlan("explain_limit_agg_pushdown2.yaml");
632+
assertYamlEqualsJsonIgnoreId(
633633
expected,
634634
explainQueryToString(
635635
"source=opensearch-sql_test_index_account | stats count() by state | head 100"));

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ public class CalcitePPLExplainIT extends PPLIntegTestCase {
1616

1717
@Override
1818
public void init() throws Exception {
19+
GlobalPushdownConfig.enabled = false;
1920
super.init();
2021
enableCalcite();
2122

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

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,6 @@ public void testPushdownSortPlusExpression() throws IOException {
3939
Locale.ROOT,
4040
"source=%s | eval age2 = age + 2 | sort age2 | fields age | head 2",
4141
TEST_INDEX_BANK);
42-
String explained = explainQueryToString(ppl);
43-
if (!isPushdownDisabled()) {
44-
assertTrue(
45-
explained.contains(
46-
"[SORT->[{\\n"
47-
+ " \\\"age\\\" : {\\n"
48-
+ " \\\"order\\\" : \\\"asc\\\",\\n"
49-
+ " \\\"missing\\\" : \\\"_first\\\"\\n"
50-
+ " }\\n"
51-
+ "}]"));
52-
}
53-
5442
JSONObject result = executeQuery(ppl);
5543
verifyOrder(result, rows(28), rows(32));
5644
}
@@ -62,18 +50,6 @@ public void testPushdownSortMinusExpression() throws IOException {
6250
Locale.ROOT,
6351
"source=%s | eval age2 = 1 - age | sort age2 | fields age | head 2",
6452
TEST_INDEX_BANK);
65-
String explained = explainQueryToString(ppl);
66-
if (!isPushdownDisabled()) {
67-
assertTrue(
68-
explained.contains(
69-
"[SORT->[{\\n"
70-
+ " \\\"age\\\" : {\\n"
71-
+ " \\\"order\\\" : \\\"desc\\\",\\n"
72-
+ " \\\"missing\\\" : \\\"_first\\\"\\n"
73-
+ " }\\n"
74-
+ "}]"));
75-
}
76-
7753
JSONObject result = executeQuery(ppl);
7854
verifyOrder(result, rows(39), rows(36));
7955
}
@@ -85,18 +61,6 @@ public void testPushdownSortTimesExpression() throws IOException {
8561
Locale.ROOT,
8662
"source=%s | eval age2 = 5 * age | sort age2 | fields age | head 2",
8763
TEST_INDEX_BANK);
88-
String explained = explainQueryToString(ppl);
89-
if (!isPushdownDisabled()) {
90-
assertTrue(
91-
explained.contains(
92-
"[SORT->[{\\n"
93-
+ " \\\"age\\\" : {\\n"
94-
+ " \\\"order\\\" : \\\"asc\\\",\\n"
95-
+ " \\\"missing\\\" : \\\"_first\\\"\\n"
96-
+ " }\\n"
97-
+ "}]"));
98-
}
99-
10064
JSONObject result = executeQuery(ppl);
10165
verifyOrder(result, rows(28), rows(32));
10266
}
@@ -108,23 +72,6 @@ public void testPushdownSortByMultiExpressions() throws IOException {
10872
Locale.ROOT,
10973
"source=%s | eval age2 = 5 * age | sort gender, age2 | fields gender, age | head 2",
11074
TEST_INDEX_BANK);
111-
String explained = explainQueryToString(ppl);
112-
if (!isPushdownDisabled()) {
113-
assertTrue(
114-
explained.contains(
115-
"[SORT->[{\\n"
116-
+ " \\\"gender.keyword\\\" : {\\n"
117-
+ " \\\"order\\\" : \\\"asc\\\",\\n"
118-
+ " \\\"missing\\\" : \\\"_first\\\"\\n"
119-
+ " }\\n"
120-
+ "}, {\\n"
121-
+ " \\\"age\\\" : {\\n"
122-
+ " \\\"order\\\" : \\\"asc\\\",\\n"
123-
+ " \\\"missing\\\" : \\\"_first\\\"\\n"
124-
+ " }\\n"
125-
+ "}]"));
126-
}
127-
12875
JSONObject result = executeQuery(ppl);
12976
verifyOrder(result, rows("F", 28), rows("F", 34));
13077
}
@@ -136,18 +83,6 @@ public void testPushdownSortCastExpression() throws IOException {
13683
Locale.ROOT,
13784
"source=%s | eval age2 = cast(age * 5 as long) | sort age2 | fields age | head 2",
13885
TEST_INDEX_BANK);
139-
String explained = explainQueryToString(ppl);
140-
if (!isPushdownDisabled()) {
141-
assertTrue(
142-
explained.contains(
143-
"[SORT->[{\\n"
144-
+ " \\\"age\\\" : {\\n"
145-
+ " \\\"order\\\" : \\\"asc\\\",\\n"
146-
+ " \\\"missing\\\" : \\\"_first\\\"\\n"
147-
+ " }\\n"
148-
+ "}]"));
149-
}
150-
15186
JSONObject result = executeQuery(ppl);
15287
verifyOrder(result, rows(28), rows(32));
15388
}

0 commit comments

Comments
 (0)