From d7965ba681a92143325d234f6158ad559e15fd1c Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Wed, 5 Sep 2018 10:44:18 +0300 Subject: [PATCH] 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 --- .../xpack/sql/parser/CommandBuilder.java | 4 +- .../plan/logical/command/sys/SysTables.java | 7 ++-- .../logical/command/sys/SysTablesTests.java | 39 ++++++++++--------- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/CommandBuilder.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/CommandBuilder.java index e3e7f9d9c5e..f2512672c6a 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/CommandBuilder.java +++ b/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 diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTables.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTables.java index 483e8a93535..69d0ad50648 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTables.java +++ b/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> values = new ArrayList<>(); // send only the types, everything else null for (IndexType type : IndexType.VALID) { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTablesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTablesTests.java index e42ec51b425..c6e1c389bb1 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTablesTests.java +++ b/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 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]); }