Browse Source

SQL: Align SYS TABLE for ODBC SQL_ALL_* args (#33364)

Fix a bug in SYS TABLES command that did skipped SQL_ALL_* arguments for
catalog and table types

Fix #33312
Costin Leau 7 years ago
parent
commit
d7965ba681

+ 2 - 2
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/CommandBuilder.java

@@ -157,9 +157,9 @@ abstract class CommandBuilder extends LogicalPlanBuilder {
             if (value != null) {
                 // check special ODBC wildcard case
                 if (value.equals(StringUtils.SQL_WILDCARD) && ctx.string().size() == 1) {
-                    // since % is the same as not specifying a value, choose
+                    // convert % to enumeration
                     // https://docs.microsoft.com/en-us/sql/odbc/reference/develop-app/value-list-arguments?view=ssdt-18vs2017
-                    // that is skip the value
+                    types.addAll(IndexType.VALID);
                 }
                 // special case for legacy apps (like msquery) that always asks for 'TABLE'
                 // which we manually map to all concrete tables supported

+ 4 - 3
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTables.java

@@ -78,7 +78,7 @@ public class SysTables extends Command {
         // https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqltables-function?view=ssdt-18vs2017#comments
 
         if (clusterPattern != null && clusterPattern.pattern().equals(SQL_WILDCARD)) {
-            if (pattern != null && pattern.pattern().isEmpty() && CollectionUtils.isEmpty(types)) {
+            if ((pattern == null || pattern.pattern().isEmpty()) && CollectionUtils.isEmpty(types)) {
                 Object[] enumeration = new Object[10];
                 // send only the cluster, everything else null
                 enumeration[0] = cluster;
@@ -88,8 +88,9 @@ public class SysTables extends Command {
         }
         
         // if no types were specified (the parser takes care of the % case)
-        if (CollectionUtils.isEmpty(types)) {
-            if (clusterPattern != null && clusterPattern.pattern().isEmpty()) {
+        if (IndexType.VALID.equals(types)) {
+            if ((clusterPattern == null || clusterPattern.pattern().isEmpty())
+                    && (pattern == null || pattern.pattern().isEmpty())) {
                 List<List<?>> values = new ArrayList<>();
                 // send only the types, everything else null
                 for (IndexType type : IndexType.VALID) {

+ 20 - 19
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTablesTests.java

@@ -49,6 +49,22 @@ public class SysTablesTests extends ESTestCase {
     private final IndexInfo index = new IndexInfo("test", IndexType.INDEX);
     private final IndexInfo alias = new IndexInfo("alias", IndexType.ALIAS);
 
+    public void testSysTablesEnumerateCatalog() throws Exception {
+        executeCommand("SYS TABLES CATALOG LIKE '%'", r -> {
+            assertEquals(1, r.size());
+            assertEquals(CLUSTER_NAME, r.column(0));
+        });
+    }
+
+    public void testSysTablesEnumerateTypes() throws Exception {
+        executeCommand("SYS TABLES TYPE '%'", r -> {
+            assertEquals(2, r.size());
+            assertEquals("ALIAS", r.column(3));
+            assertTrue(r.advanceRow());
+            assertEquals("BASE TABLE", r.column(3));
+        });
+    }
+
     public void testSysTablesDifferentCatalog() throws Exception {
         executeCommand("SYS TABLES CATALOG LIKE 'foo'", r -> {
             assertEquals(0, r.size());
@@ -58,10 +74,10 @@ public class SysTablesTests extends ESTestCase {
 
     public void testSysTablesNoTypes() throws Exception {
         executeCommand("SYS TABLES", r -> {
-            assertEquals("alias", r.column(2));
-            assertTrue(r.advanceRow());
             assertEquals(2, r.size());
-            assertEquals("test", r.column(2));
+            assertEquals("ALIAS", r.column(3));
+            assertTrue(r.advanceRow());
+            assertEquals("BASE TABLE", r.column(3));
         }, index, alias);
     }
 
@@ -208,22 +224,7 @@ public class SysTablesTests extends ESTestCase {
 
     public void testSysTablesTypesEnumerationWoString() throws Exception {
         executeCommand("SYS TABLES CATALOG LIKE '' LIKE '' ", r -> {
-            assertEquals(2, r.size());
-
-            Iterator<IndexType> it = IndexType.VALID.stream().sorted(Comparator.comparing(IndexType::toSql)).iterator();
-
-            for (int t = 0; t < r.size(); t++) {
-                assertEquals(it.next().toSql(), r.column(3));
-
-                // everything else should be null
-                for (int i = 0; i < 10; i++) {
-                    if (i != 3) {
-                        assertNull(r.column(i));
-                    }
-                }
-
-                r.advanceRow();
-            }
+            assertEquals(0, r.size());
         }, new IndexInfo[0]);
     }