From bc6540b309d696600a7960f1cbc570d2e495a5fb Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Thu, 29 Sep 2022 00:38:01 -0700 Subject: [PATCH 1/2] Do not remove opensearch from the list of registered catalogs. Signed-off-by: MaxKsyunz --- .../sql/plugin/catalog/CatalogServiceImpl.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java b/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java index 5a77961d8b..561bc35ec1 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -86,9 +87,9 @@ public StorageEngine getStorageEngine(String catalog) { @Override public Set getCatalogs() { - Set catalogs = storageEngineMap.keySet(); - catalogs.remove(OPEN_SEARCH); - return catalogs; + return storageEngineMap.keySet() + .stream().filter(catalog -> !catalog.equals(OPEN_SEARCH)) + .collect(Collectors.toSet()); } @Override @@ -165,4 +166,4 @@ private void validateCatalogs(List catalogs) { } -} \ No newline at end of file +} From 9da8fb181969772febf6297708dbf997065724ab Mon Sep 17 00:00:00 2001 From: vamsi-amazon Date: Thu, 29 Sep 2022 11:51:34 -0700 Subject: [PATCH 2/2] New Changes to handle bug:https://github.com/opensearch-project/sql/issues/866 Signed-off-by: vamsi-amazon --- .../org/opensearch/sql/sql/IdentifierIT.java | 7 +++++++ .../plugin/catalog/CatalogServiceImpl.java | 15 +++++++------ .../catalog/CatalogServiceImplTest.java | 21 ++++++++++++++++++- plugin/src/test/resources/empty_catalog.json | 1 + 4 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 plugin/src/test/resources/empty_catalog.json diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/IdentifierIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/IdentifierIT.java index 29e07075e1..591364ea19 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/IdentifierIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/IdentifierIT.java @@ -57,6 +57,13 @@ public void testSpecialFieldName() throws IOException { verifyDataRows(result, rows(10, 30)); } + @Test + public void testMultipleQueriesWithSpecialIndexNames() throws IOException { + createIndexWithOneDoc("test.one", "test.two"); + queryAndAssertTheDoc("SELECT * FROM test.one"); + queryAndAssertTheDoc("SELECT * FROM test.two"); + } + private void createIndexWithOneDoc(String... indexNames) throws IOException { for (String indexName : indexNames) { new Index(indexName).addDoc("{\"age\": 30}"); diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java b/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java index 561bc35ec1..47826ff644 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/catalog/CatalogServiceImpl.java @@ -8,10 +8,12 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableSet; import java.io.IOException; import java.io.InputStream; import java.net.URISyntaxException; import java.security.PrivilegedExceptionAction; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -37,7 +39,7 @@ public class CatalogServiceImpl implements CatalogService { private static final Logger LOG = LogManager.getLogger(); - public static final String OPEN_SEARCH = "opensearch"; + public static StorageEngine defaultOpenSearchStorageEngine; private Map storageEngineMap = new HashMap<>(); @@ -80,21 +82,22 @@ public void loadConnectors(Settings settings) { @Override public StorageEngine getStorageEngine(String catalog) { if (catalog == null || !storageEngineMap.containsKey(catalog)) { - return storageEngineMap.get(OPEN_SEARCH); + return defaultOpenSearchStorageEngine; } return storageEngineMap.get(catalog); } @Override public Set getCatalogs() { - return storageEngineMap.keySet() - .stream().filter(catalog -> !catalog.equals(OPEN_SEARCH)) - .collect(Collectors.toSet()); + return Collections.unmodifiableSet(storageEngineMap.keySet()); } @Override public void registerOpenSearchStorageEngine(StorageEngine storageEngine) { - storageEngineMap.put(OPEN_SEARCH, storageEngine); + if (storageEngine == null) { + throw new IllegalArgumentException("Default storage engine can't be null"); + } + defaultOpenSearchStorageEngine = storageEngine; } private T doPrivileged(PrivilegedExceptionAction action) { diff --git a/plugin/src/test/java/org/opensearch/sql/plugin/catalog/CatalogServiceImplTest.java b/plugin/src/test/java/org/opensearch/sql/plugin/catalog/CatalogServiceImplTest.java index 678962cbb5..ad892a70f5 100644 --- a/plugin/src/test/java/org/opensearch/sql/plugin/catalog/CatalogServiceImplTest.java +++ b/plugin/src/test/java/org/opensearch/sql/plugin/catalog/CatalogServiceImplTest.java @@ -15,15 +15,21 @@ import lombok.SneakyThrows; import org.junit.Assert; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; import org.opensearch.common.settings.MockSecureSettings; import org.opensearch.common.settings.Settings; +import org.opensearch.sql.storage.StorageEngine; - +@RunWith(MockitoJUnitRunner.class) public class CatalogServiceImplTest { public static final String CATALOG_SETTING_METADATA_KEY = "plugins.query.federation.catalog.config"; + @Mock + private StorageEngine storageEngine; @SneakyThrows @Test @@ -73,6 +79,19 @@ public void testLoadConnectorsWithMalformedJson() { () -> CatalogServiceImpl.getInstance().loadConnectors(settings)); } + @SneakyThrows + @Test + public void testGetStorageEngineAfterGetCatalogs() { + Settings settings = getCatalogSettings("empty_catalog.json"); + CatalogServiceImpl.getInstance().registerOpenSearchStorageEngine(storageEngine); + CatalogServiceImpl.getInstance().loadConnectors(settings); + Set expected = new HashSet<>(); + Assert.assertEquals(expected, CatalogServiceImpl.getInstance().getCatalogs()); + Assert.assertEquals(storageEngine, CatalogServiceImpl.getInstance().getStorageEngine(null)); + Assert.assertEquals(expected, CatalogServiceImpl.getInstance().getCatalogs()); + Assert.assertEquals(storageEngine, CatalogServiceImpl.getInstance().getStorageEngine(null)); + } + private Settings getCatalogSettings(String filename) throws URISyntaxException, IOException { MockSecureSettings mockSecureSettings = new MockSecureSettings(); diff --git a/plugin/src/test/resources/empty_catalog.json b/plugin/src/test/resources/empty_catalog.json new file mode 100644 index 0000000000..0637a088a0 --- /dev/null +++ b/plugin/src/test/resources/empty_catalog.json @@ -0,0 +1 @@ +[] \ No newline at end of file