Skip to content

Commit e9a009b

Browse files
Yury-FridlyandMaxKsyunz
Yury-Fridlyand
and
MaxKsyunz
authored
Backport breaking changes to 2.x (#1792)
* Update SQL plugin for core refactor (#1571) Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix plugin compilation (#1580) * Changed gradle version and removed values iterator Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Update a test to match new indexResponse.aliases() type. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Ran ./gradlew wrapper Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> --------- Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Co-authored-by: MaxKsyunz <maxk@bitquilltech.com> Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Update sqlite-jdbc to 3.41.2.2 to address CVE-2023-32697 (#1667) * Update sqlite-jdbc to 3.41.2.2 to address CVE-2023-32697 Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Don't check column names on H2 results for correctness tests as described in #1667 (comment). Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR review comment. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> Co-authored-by: MaxKsyunz <maxk@bitquilltech.com>
1 parent 6532a10 commit e9a009b

9 files changed

Lines changed: 69 additions & 68 deletions

File tree

‎integ-test/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ dependencies {
9494
testRuntimeOnly('org.junit.jupiter:junit-jupiter-engine:5.6.2')
9595

9696
testImplementation group: 'com.h2database', name: 'h2', version: '2.1.214'
97-
testImplementation group: 'org.xerial', name: 'sqlite-jdbc', version: '3.28.0'
97+
testImplementation group: 'org.xerial', name: 'sqlite-jdbc', version: '3.41.2.2'
9898
testImplementation group: 'com.google.code.gson', name: 'gson', version: '2.8.9'
9999
}
100100

‎integ-test/src/test/java/org/opensearch/sql/correctness/runner/resultset/DBResult.java‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212
import java.util.Collection;
1313
import java.util.Collections;
1414
import java.util.List;
15+
import java.util.Objects;
1516
import java.util.Set;
16-
import lombok.EqualsAndHashCode;
17+
import java.util.stream.Collectors;
1718
import lombok.Getter;
1819
import lombok.ToString;
1920
import org.json.JSONPropertyName;
@@ -24,7 +25,6 @@
2425
* query with SELECT columns or just *, order of column and row may matter or not. So the internal data structure of this
2526
* class is passed in from outside either list or set, hash map or linked hash map etc.
2627
*/
27-
@EqualsAndHashCode(exclude = "databaseName")
2828
@ToString
2929
public class DBResult {
3030

@@ -191,4 +191,24 @@ private static <T extends Comparable<T>> List<T> sort(Collection<T> collection)
191191
return list;
192192
}
193193

194+
public boolean equals(final Object o) {
195+
if (o == this) {
196+
return true;
197+
}
198+
if (!(o instanceof DBResult)) {
199+
return false;
200+
}
201+
final DBResult other = (DBResult) o;
202+
// H2 calculates the value before setting column name
203+
// for example, for query "select 1 + 1" it returns a column named "2" instead of "1 + 1"
204+
boolean skipColumnNameCheck = databaseName.equalsIgnoreCase("h2") || other.databaseName.equalsIgnoreCase("h2");
205+
if (!skipColumnNameCheck && !schema.equals(other.schema)) {
206+
return false;
207+
}
208+
if (skipColumnNameCheck && !schema.stream().map(Type::getType).collect(Collectors.toList())
209+
.equals(other.schema.stream().map(Type::getType).collect(Collectors.toList()))) {
210+
return false;
211+
}
212+
return dataRows.equals(other.dataRows);
213+
}
194214
}

‎legacy/src/main/java/org/opensearch/sql/legacy/esdomain/mapping/IndexMappings.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
import java.util.Objects;
1313
import org.opensearch.cluster.metadata.MappingMetadata;
1414
import org.opensearch.cluster.metadata.Metadata;
15-
import org.opensearch.common.collect.ImmutableOpenMap;
16-
import org.opensearch.sql.legacy.domain.Field;
1715

1816
/**
1917
* Index mappings in the cluster.
@@ -51,7 +49,7 @@ public IndexMappings(Metadata metaData) {
5149
indexMetaData -> new FieldMappings(indexMetaData.mapping()));
5250
}
5351

54-
public IndexMappings(ImmutableOpenMap<String, MappingMetadata> mappings) {
52+
public IndexMappings(Map<String, MappingMetadata> mappings) {
5553
this.indexMappings = buildMappings(mappings, FieldMappings::new);
5654
}
5755

‎legacy/src/main/java/org/opensearch/sql/legacy/esdomain/mapping/Mappings.java‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,10 @@
66

77
package org.opensearch.sql.legacy.esdomain.mapping;
88

9-
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
10-
import com.google.common.collect.ImmutableMap;
119
import java.util.Collection;
1210
import java.util.Map;
1311
import java.util.function.Function;
14-
import org.opensearch.common.collect.ImmutableOpenMap;
12+
import java.util.stream.Collectors;
1513

1614
/**
1715
* Mappings interface to provide default implementation (minimal set of Map methods) for subclass in hierarchy.
@@ -47,13 +45,10 @@ default boolean isEmpty() {
4745
Map<String, T> data();
4846

4947
/**
50-
* Convert OpenSearch ImmutableOpenMap<String, X> to JDK Map<String, Y> by applying function: Y func(X)
48+
* Build a map from an existing map by applying provided function to each value.
5149
*/
52-
default <X, Y> Map<String, Y> buildMappings(ImmutableOpenMap<String, X> mappings, Function<X, Y> func) {
53-
ImmutableMap.Builder<String, Y> builder = ImmutableMap.builder();
54-
for (ObjectObjectCursor<String, X> mapping : mappings) {
55-
builder.put(mapping.key, func.apply(mapping.value));
56-
}
57-
return builder.build();
50+
default <X, Y> Map<String, Y> buildMappings(Map<String, X> mappings, Function<X, Y> func) {
51+
return mappings.entrySet().stream().collect(
52+
Collectors.toUnmodifiableMap(Map.Entry::getKey, func.compose(Map.Entry::getValue)));
5853
}
5954
}

‎legacy/src/main/java/org/opensearch/sql/legacy/executor/format/DescribeResultSet.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
package org.opensearch.sql.legacy.executor.format;
88

9-
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
109
import java.util.ArrayList;
1110
import java.util.HashMap;
1211
import java.util.List;
@@ -15,7 +14,6 @@
1514
import org.opensearch.action.admin.indices.get.GetIndexResponse;
1615
import org.opensearch.client.Client;
1716
import org.opensearch.cluster.metadata.MappingMetadata;
18-
import org.opensearch.common.collect.ImmutableOpenMap;
1917
import org.opensearch.sql.legacy.domain.IndexStatement;
2018
import org.opensearch.sql.legacy.executor.format.DataRows.Row;
2119
import org.opensearch.sql.legacy.executor.format.Schema.Column;
@@ -79,14 +77,14 @@ private List<Column> loadColumns() {
7977
private List<Row> loadRows() {
8078
List<Row> rows = new ArrayList<>();
8179
GetIndexResponse indexResponse = (GetIndexResponse) queryResult;
82-
ImmutableOpenMap<String, MappingMetadata> indexMappings = indexResponse.getMappings();
80+
Map<String, MappingMetadata> indexMappings = indexResponse.getMappings();
8381

8482
// Iterate through indices in indexMappings
85-
for (ObjectObjectCursor<String, MappingMetadata> indexCursor : indexMappings) {
86-
String index = indexCursor.key;
83+
for (Entry<String, MappingMetadata> indexCursor : indexMappings.entrySet()) {
84+
String index = indexCursor.getKey();
8785

8886
if (matchesPatternIfRegex(index, statement.getIndexPattern())) {
89-
rows.addAll(loadIndexData(index, indexCursor.value.getSourceAsMap()));
87+
rows.addAll(loadIndexData(index, indexCursor.getValue().getSourceAsMap()));
9088
}
9189
}
9290
return rows;

‎legacy/src/test/java/org/opensearch/sql/legacy/util/CheckScriptContents.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.io.IOException;
2424
import java.sql.SQLFeatureNotSupportedException;
2525
import java.util.List;
26+
import java.util.Map;
2627
import java.util.regex.Matcher;
2728
import java.util.regex.Pattern;
2829
import org.mockito.stubbing.Answer;
@@ -227,9 +228,9 @@ public static ClusterService mockClusterService(String mappings) {
227228
when(mockService.state()).thenReturn(mockState);
228229
when(mockState.metadata()).thenReturn(mockMetaData);
229230
try {
230-
ImmutableOpenMap.Builder<String, MappingMetadata> builder = ImmutableOpenMap.builder();
231-
builder.put(TestsConstants.TEST_INDEX_BANK, IndexMetadata.fromXContent(createParser(mappings)).mapping());
232-
when(mockMetaData.findMappings(any(), any())).thenReturn(builder.build());
231+
when(mockMetaData.findMappings(any(), any())).thenReturn(
232+
Map.of(TestsConstants.TEST_INDEX_BANK, IndexMetadata.fromXContent(
233+
createParser(mappings)).mapping()));
233234
}
234235
catch (IOException e) {
235236
throw new IllegalStateException(e);

‎legacy/src/test/java/org/opensearch/sql/legacy/util/MultipleIndexClusterUtils.java‎

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,14 @@
1414
import static org.opensearch.sql.legacy.util.CheckScriptContents.mockIndexNameExpressionResolver;
1515
import static org.opensearch.sql.legacy.util.CheckScriptContents.mockPluginSettings;
1616

17-
import com.google.common.collect.ImmutableMap;
1817
import java.io.IOException;
1918
import java.util.Map;
19+
import java.util.stream.Collectors;
2020
import org.opensearch.cluster.ClusterState;
2121
import org.opensearch.cluster.metadata.IndexMetadata;
2222
import org.opensearch.cluster.metadata.MappingMetadata;
2323
import org.opensearch.cluster.metadata.Metadata;
2424
import org.opensearch.cluster.service.ClusterService;
25-
import org.opensearch.common.collect.ImmutableOpenMap;
2625
import org.opensearch.sql.legacy.esdomain.LocalClusterState;
2726

2827
/**
@@ -139,57 +138,54 @@ public class MultipleIndexClusterUtils {
139138
"}";
140139

141140
public static void mockMultipleIndexEnv() {
142-
mockLocalClusterState(new ImmutableMap.Builder<String, ImmutableOpenMap<String, MappingMetadata>>()
143-
.put(INDEX_ACCOUNT_1, buildIndexMapping(INDEX_ACCOUNT_1, INDEX_ACCOUNT_1_MAPPING))
144-
.put(INDEX_ACCOUNT_2, buildIndexMapping(INDEX_ACCOUNT_2, INDEX_ACCOUNT_2_MAPPING))
145-
.put(INDEX_ACCOUNT_ALL, buildIndexMapping(new ImmutableMap.Builder<String, String>()
146-
.put(INDEX_ACCOUNT_1, INDEX_ACCOUNT_1_MAPPING)
147-
.put(INDEX_ACCOUNT_2, INDEX_ACCOUNT_2_MAPPING)
148-
.build()))
149-
.build());
141+
mockLocalClusterState(
142+
Map.of(INDEX_ACCOUNT_1, buildIndexMapping(INDEX_ACCOUNT_1, INDEX_ACCOUNT_1_MAPPING),
143+
INDEX_ACCOUNT_2, buildIndexMapping(INDEX_ACCOUNT_2, INDEX_ACCOUNT_2_MAPPING),
144+
INDEX_ACCOUNT_ALL, buildIndexMapping(Map.of(INDEX_ACCOUNT_1, INDEX_ACCOUNT_1_MAPPING,
145+
INDEX_ACCOUNT_2, INDEX_ACCOUNT_2_MAPPING))));
150146
}
151147

152-
public static void mockLocalClusterState(Map<String, ImmutableOpenMap<String,MappingMetadata>> indexMapping) {
148+
public static void mockLocalClusterState(Map<String, Map<String,MappingMetadata>> indexMapping) {
153149
LocalClusterState.state().setClusterService(mockClusterService(indexMapping));
154150
LocalClusterState.state().setResolver(mockIndexNameExpressionResolver());
155151
LocalClusterState.state().setPluginSettings(mockPluginSettings());
156152
}
157153

158154

159-
public static ClusterService mockClusterService(Map<String, ImmutableOpenMap<String,MappingMetadata>> indexMapping) {
155+
public static ClusterService mockClusterService(Map<String, Map<String,MappingMetadata>>
156+
indexMapping) {
160157
ClusterService mockService = mock(ClusterService.class);
161158
ClusterState mockState = mock(ClusterState.class);
162159
Metadata mockMetaData = mock(Metadata.class);
163160

164161
when(mockService.state()).thenReturn(mockState);
165162
when(mockState.metadata()).thenReturn(mockMetaData);
166163
try {
167-
for (Map.Entry<String, ImmutableOpenMap<String, MappingMetadata>> entry : indexMapping.entrySet()) {
168-
when(mockMetaData.findMappings(eq(new String[]{entry.getKey()}), any())).thenReturn(entry.getValue());
164+
for (var entry : indexMapping.entrySet()) {
165+
when(mockMetaData.findMappings(eq(new String[]{entry.getKey()}), any()))
166+
.thenReturn(entry.getValue());
169167
}
170168
} catch (IOException e) {
171169
throw new IllegalStateException(e);
172170
}
173171
return mockService;
174172
}
175173

176-
private static ImmutableOpenMap<String, MappingMetadata> buildIndexMapping(Map<String, String> indexMapping) {
177-
try {
178-
ImmutableOpenMap.Builder<String, MappingMetadata> builder = ImmutableOpenMap.builder();
179-
for (Map.Entry<String, String> entry : indexMapping.entrySet()) {
180-
builder.put(entry.getKey(), IndexMetadata.fromXContent(createParser(entry.getValue())).mapping());
174+
private static Map<String, MappingMetadata> buildIndexMapping(Map<String, String> indexMapping) {
175+
return indexMapping.entrySet().stream().collect(Collectors.toUnmodifiableMap(
176+
Map.Entry::getKey, e -> {
177+
try {
178+
return IndexMetadata.fromXContent(createParser(e.getValue())).mapping();
179+
} catch (IOException ex) {
180+
throw new IllegalStateException(ex);
181181
}
182-
return builder.build();
183-
} catch (IOException e) {
184-
throw new IllegalStateException(e);
185-
}
182+
}));
183+
186184
}
187185

188-
private static ImmutableOpenMap<String, MappingMetadata> buildIndexMapping(String index, String mapping) {
186+
private static Map<String, MappingMetadata> buildIndexMapping(String index, String mapping) {
189187
try {
190-
ImmutableOpenMap.Builder<String, MappingMetadata> builder = ImmutableOpenMap.builder();
191-
builder.put(index, IndexMetadata.fromXContent(createParser(mapping)).mapping());
192-
return builder.build();
188+
return Map.of(index, IndexMetadata.fromXContent(createParser(mapping)).mapping());
193189
} catch (IOException e) {
194190
throw new IllegalStateException(e);
195191
}

‎opensearch/src/main/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClient.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
1010
import com.google.common.collect.ImmutableList;
1111
import com.google.common.collect.ImmutableMap;
12-
import com.google.common.collect.Streams;
1312
import java.util.Arrays;
1413
import java.util.Collection;
1514
import java.util.List;
@@ -89,9 +88,9 @@ public Map<String, IndexMapping> getIndexMappings(String... indexExpression) {
8988
.prepareGetMappings(indexExpression)
9089
.setLocal(true)
9190
.get();
92-
return Streams.stream(mappingsResponse.mappings().iterator())
93-
.collect(Collectors.toMap(cursor -> cursor.key,
94-
cursor -> new IndexMapping(cursor.value)));
91+
return mappingsResponse.mappings().entrySet().stream().collect(Collectors.toUnmodifiableMap(
92+
Map.Entry::getKey,
93+
cursor -> new IndexMapping(cursor.getValue())));
9594
} catch (IndexNotFoundException e) {
9695
// Re-throw directly to be treated as client error finally
9796
throw e;
@@ -152,7 +151,7 @@ public List<String> indices() {
152151
.setLocal(true)
153152
.get();
154153
final Stream<String> aliasStream =
155-
ImmutableList.copyOf(indexResponse.aliases().valuesIt()).stream()
154+
ImmutableList.copyOf(indexResponse.aliases().values()).stream()
156155
.flatMap(Collection::stream).map(AliasMetadata::alias);
157156

158157
return Stream.concat(Arrays.stream(indexResponse.getIndices()), aliasStream)

‎opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -399,9 +399,7 @@ void cleanup_rethrows_exception() {
399399
@Test
400400
void get_indices() {
401401
AliasMetadata aliasMetadata = mock(AliasMetadata.class);
402-
ImmutableOpenMap.Builder<String, List<AliasMetadata>> builder = ImmutableOpenMap.builder();
403-
builder.fPut("index", Arrays.asList(aliasMetadata));
404-
final ImmutableOpenMap<String, List<AliasMetadata>> openMap = builder.build();
402+
final var openMap = Map.of("index", List.of(aliasMetadata));
405403
when(aliasMetadata.alias()).thenReturn("index_alias");
406404
when(nodeClient.admin().indices()
407405
.prepareGetIndex()
@@ -437,16 +435,12 @@ public void mockNodeClientIndicesMappings(String indexName, String mappings) {
437435
.setLocal(anyBoolean())
438436
.get()).thenReturn(mockResponse);
439437
try {
440-
ImmutableOpenMap<String, MappingMetadata> metadata;
438+
Map<String, MappingMetadata> metadata;
441439
if (mappings.isEmpty()) {
442-
when(emptyMapping.getSourceAsMap()).thenReturn(ImmutableMap.of());
443-
metadata =
444-
new ImmutableOpenMap.Builder<String, MappingMetadata>()
445-
.fPut(indexName, emptyMapping)
446-
.build();
440+
when(emptyMapping.getSourceAsMap()).thenReturn(Map.of());
441+
metadata = Map.of(indexName, emptyMapping);
447442
} else {
448-
metadata = new ImmutableOpenMap.Builder<String, MappingMetadata>().fPut(indexName,
449-
IndexMetadata.fromXContent(createParser(mappings)).mapping()).build();
443+
metadata = Map.of(indexName, IndexMetadata.fromXContent(createParser(mappings)).mapping());
450444
}
451445
when(mockResponse.mappings()).thenReturn(metadata);
452446
} catch (IOException e) {

0 commit comments

Comments
 (0)