Skip to content

Commit 87b28b2

Browse files
xinyualxinyual
committed
refactor merge rules
Signed-off-by: xinyual <xinyual@amazon.com>
1 parent 9e65b49 commit 87b28b2

13 files changed

Lines changed: 197 additions & 85 deletions

File tree

‎integ-test/src/test/java/org/opensearch/sql/calcite/standalone/CalcitePPLBasicIT.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,14 +571,22 @@ public void testFieldsMergedObject() {
571571
executeQuery(
572572
String.format(
573573
"source=%s | fields machine.os1, machine.os2, machine_array.os1, "
574-
+ " machine_array.os2",
574+
+ " machine_array.os2, machine_deep.attr1, machine_deep.attr2,"
575+
+ " machine_deep.layer.os1, machine_deep.layer.os2",
575576
TEST_INDEX_MERGE_TEST_WILDCARD));
576577
verifySchema(
577578
result,
578579
schema("machine.os1", "string"),
579580
schema("machine.os2", "string"),
580581
schema("machine_array.os1", "string"),
581-
schema("machine_array.os2", "string"));
582-
verifyDataRows(result, rows("linux", null, "linux", null), rows(null, "linux", null, "linux"));
582+
schema("machine_array.os2", "string"),
583+
schema("machine_deep.attr1", "long"),
584+
schema("machine_deep.attr2", "long"),
585+
schema("machine_deep.layer.os1", "string"),
586+
schema("machine_deep.layer.os2", "string"));
587+
verifyDataRows(
588+
result,
589+
rows("linux", null, "linux", null, 1, null, "os1", null),
590+
rows(null, "linux", null, "linux", null, 2, null, "os2"));
583591
}
584592
}

‎integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,19 +108,27 @@ public void testMetadataFieldsWithEvalMetaField() {
108108
}
109109

110110
@Test
111-
public void testFieldsTwoMergedObject() throws IOException {
111+
public void testFieldsMergedObject() throws IOException {
112112
JSONObject result =
113113
executeQuery(
114114
String.format(
115115
"source=%s | fields machine.os1, machine.os2, machine_array.os1, "
116-
+ " machine_array.os2",
116+
+ " machine_array.os2, machine_deep.attr1, machine_deep.attr2,"
117+
+ " machine_deep.layer.os1, machine_deep.layer.os2",
117118
TEST_INDEX_MERGE_TEST_WILDCARD));
118119
verifySchema(
119120
result,
120121
schema("machine.os1", "string"),
121122
schema("machine.os2", "string"),
122123
schema("machine_array.os1", "string"),
123-
schema("machine_array.os2", "string"));
124-
verifyDataRows(result, rows("linux", null, "linux", null), rows(null, "linux", null, "linux"));
124+
schema("machine_array.os2", "string"),
125+
schema("machine_deep.attr1", "bigint"),
126+
schema("machine_deep.attr2", "bigint"),
127+
schema("machine_deep.layer.os1", "string"),
128+
schema("machine_deep.layer.os2", "string"));
129+
verifyDataRows(
130+
result,
131+
rows("linux", null, "linux", null, 1, null, "os1", null),
132+
rows(null, "linux", null, "linux", null, 2, null, "os2"));
125133
}
126134
}

‎integ-test/src/test/resources/indexDefinitions/merge_test_1_mapping.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,20 @@
1111
}
1212
}
1313
},
14+
"machine_deep": {
15+
"properties": {
16+
"attr1": {
17+
"type": "long"
18+
},
19+
"layer": {
20+
"properties": {
21+
"os1": {
22+
"type": "text"
23+
}
24+
}
25+
}
26+
}
27+
},
1428
"machine_array": {
1529
"type": "nested",
1630
"properties": {

‎integ-test/src/test/resources/indexDefinitions/merge_test_2_mapping.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,20 @@
1111
}
1212
}
1313
},
14+
"machine_deep": {
15+
"properties": {
16+
"attr2": {
17+
"type": "long"
18+
},
19+
"layer": {
20+
"properties": {
21+
"os2": {
22+
"type": "text"
23+
}
24+
}
25+
}
26+
}
27+
},
1428
"machine_array": {
1529
"type": "nested",
1630
"properties": {

‎integ-test/src/test/resources/merge_test_1.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
{"index":{"_id":"1"}}
2-
{"machine": {"os1": "linux", "ram1": 120}, "machine_array": [{"os1": "linux", "ram1": 120}]}
2+
{"machine": {"os1": "linux", "ram1": 120}, "machine_array": [{"os1": "linux", "ram1": 120}], "machine_deep": {"attr1": 1, "layer": {"os1": "os1"}}}

‎integ-test/src/test/resources/merge_test_2.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
{"index": {}}
2-
{"machine": {"os2": "linux", "ram2": 120}, "machine_array": [{"os2": "linux", "ram2": 120}]}
2+
{"machine": {"os2": "linux", "ram2": 120}, "machine_array": [{"os2": "linux", "ram2": 120}],"machine_deep": {"attr2": 2, "layer": {"os2": "os2"}}}

‎opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java‎

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

88
import static org.opensearch.sql.data.model.ExprValueUtils.*;
99
import static org.opensearch.sql.opensearch.client.OpenSearchClient.META_CLUSTER_NAME;
10-
import static org.opensearch.sql.opensearch.util.MergeRuleUtils.checkWhetherToMerge;
1110

1211
import java.util.ArrayList;
1312
import java.util.Arrays;
@@ -25,6 +24,7 @@
2524
import org.opensearch.sql.opensearch.data.type.OpenSearchDataType;
2625
import org.opensearch.sql.opensearch.mapping.IndexMapping;
2726
import org.opensearch.sql.opensearch.request.OpenSearchRequest;
27+
import org.opensearch.sql.opensearch.util.MergeRules.MergeRuleHelper;
2828

2929
@Log4j2
3030
/** Describe index meta data request. */
@@ -108,7 +108,7 @@ public Map<String, OpenSearchDataType> getFieldTypes() {
108108
}
109109
} else {
110110
for (IndexMapping indexMapping : indexMappings.values()) {
111-
mergeObjectAndArrayInsideMap(fieldTypes, indexMapping.getFieldMappings());
111+
MergeRuleHelper.merge(fieldTypes, indexMapping.getFieldMappings());
112112
}
113113
}
114114
return fieldTypes;
@@ -165,29 +165,6 @@ private String clusterName(Map<String, String> meta) {
165165
return meta.getOrDefault(META_CLUSTER_NAME, DEFAULT_TABLE_CAT);
166166
}
167167

168-
/**
169-
* The function accept two map and merge them. It will merge object/nested DataType if they're
170-
* under same key
171-
*
172-
* @param target The target map we will merge into
173-
* @param source The candidate map
174-
*/
175-
public static void mergeObjectAndArrayInsideMap(
176-
Map<String, OpenSearchDataType> target, Map<String, OpenSearchDataType> source) {
177-
for (Map.Entry<String, OpenSearchDataType> entry : source.entrySet()) {
178-
String key = entry.getKey();
179-
OpenSearchDataType value = entry.getValue();
180-
181-
if (target.containsKey(key) && checkWhetherToMerge(value, target.get(key))) {
182-
OpenSearchDataType merged = target.get(key);
183-
mergeObjectAndArrayInsideMap(merged.getProperties(), value.getProperties());
184-
target.put(key, merged);
185-
} else {
186-
target.put(key, value);
187-
}
188-
}
189-
}
190-
191168
@Override
192169
public String toString() {
193170
return "OpenSearchDescribeIndexRequest{indexName='" + indexName + "\'}";

‎opensearch/src/main/java/org/opensearch/sql/opensearch/util/MergeRuleUtils.java‎

Lines changed: 0 additions & 51 deletions
This file was deleted.

‎opensearch/src/main/java/org/opensearch/sql/opensearch/util/MergeRules/DeepMergeRule.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.opensearch.util.MergeRules;
7+
8+
import java.util.Map;
9+
import org.opensearch.sql.data.type.ExprCoreType;
10+
import org.opensearch.sql.opensearch.data.type.OpenSearchDataType;
11+
12+
/** This rule will merge two array/struct object and merge their properties */
13+
public class DeepMergeRule implements MergeRule {
14+
15+
@Override
16+
public boolean isMatch(OpenSearchDataType source, OpenSearchDataType target) {
17+
return source != null
18+
&& target != null
19+
&& source.getExprCoreType() == target.getExprCoreType()
20+
&& (source.getExprCoreType() == ExprCoreType.STRUCT
21+
|| source.getExprCoreType() == ExprCoreType.ARRAY);
22+
}
23+
24+
@Override
25+
public void mergeInto(
26+
String key, OpenSearchDataType source, Map<String, OpenSearchDataType> target) {
27+
OpenSearchDataType existing = target.get(key);
28+
MergeRuleHelper.merge(existing.getProperties(), source.getProperties());
29+
target.put(key, existing);
30+
}
31+
}

‎opensearch/src/main/java/org/opensearch/sql/opensearch/util/MergeRules/LatestRule.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.opensearch.util.MergeRules;
7+
8+
import java.util.Map;
9+
import org.opensearch.sql.opensearch.data.type.OpenSearchDataType;
10+
11+
/** The rule always keep the latest one. */
12+
public class LatestRule implements MergeRule {
13+
14+
@Override
15+
public boolean isMatch(OpenSearchDataType source, OpenSearchDataType target) {
16+
return true;
17+
}
18+
19+
@Override
20+
public void mergeInto(
21+
String key, OpenSearchDataType source, Map<String, OpenSearchDataType> target) {
22+
target.put(key, source);
23+
}
24+
}

0 commit comments

Comments
 (0)