From b5dd4c649a9ea7624957f71bf12d5065257dfab4 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Fri, 2 Feb 2018 17:50:33 +0200 Subject: [PATCH] SQL: Minor fixes Original commit: elastic/x-pack-elasticsearch@14ea747e20f50cd6037bc9ce94f0407308d084bf --- .../authz/AuthorizationServiceTests.java | 6 ++-- .../xpack/sql/jdbc/debug/DebugProxy.java | 4 ++- .../sql/jdbc/jdbc/JdbcConfiguration.java | 21 ++++++----- .../xpack/sql/jdbc/jdbc/JdbcConnection.java | 2 -- .../sql/jdbc/jdbc/JdbcDatabaseMetaData.java | 3 +- .../xpack/sql/plan/logical/EsRelation.java | 35 ++++++++++++++++-- .../xpack/sql/util/StringUtils.java | 36 ------------------- 7 files changed, 50 insertions(+), 57 deletions(-) diff --git a/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index 17e161727d7..e7395f64a1e 100644 --- a/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -136,6 +136,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import static java.util.Arrays.asList; import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException; import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationException; import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationExceptionRunAs; @@ -341,11 +342,10 @@ public class AuthorizationServiceTests extends ESTestCase { } public void testUnknownRoleCausesDenial() { - @SuppressWarnings("unchecked") - Tuple tuple = randomFrom( + Tuple tuple = randomFrom(asList( new Tuple<>(SearchAction.NAME, new SearchRequest()), new Tuple<>(IndicesExistsAction.NAME, new IndicesExistsRequest()), - new Tuple<>(SqlQueryAction.NAME, new SqlQueryRequest())); + new Tuple<>(SqlQueryAction.NAME, new SqlQueryRequest()))); String action = tuple.v1(); TransportRequest request = tuple.v2(); User user = new User("test user", "non-existent-role"); diff --git a/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/debug/DebugProxy.java b/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/debug/DebugProxy.java index 9294075cb1d..90d775f31a7 100644 --- a/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/debug/DebugProxy.java +++ b/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/debug/DebugProxy.java @@ -5,7 +5,9 @@ */ package org.elasticsearch.xpack.sql.jdbc.debug; -// Debug marker interface for compatible proxy. +/** + * Debug marker interface for compatible proxy. + */ interface DebugProxy { } diff --git a/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConfiguration.java b/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConfiguration.java index ffa63ce3da4..bdbc062351f 100644 --- a/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConfiguration.java +++ b/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConfiguration.java @@ -7,8 +7,8 @@ package org.elasticsearch.xpack.sql.jdbc.jdbc; import org.elasticsearch.xpack.sql.client.shared.ConnectionConfiguration; import org.elasticsearch.xpack.sql.client.shared.StringUtils; -import org.elasticsearch.xpack.sql.jdbc.JdbcSQLException; import org.elasticsearch.xpack.sql.client.shared.Version; +import org.elasticsearch.xpack.sql.jdbc.JdbcSQLException; import java.net.URI; import java.sql.DriverPropertyInfo; @@ -25,16 +25,15 @@ import java.util.concurrent.TimeUnit; import static org.elasticsearch.xpack.sql.client.shared.UriUtils.parseURI; import static org.elasticsearch.xpack.sql.client.shared.UriUtils.removeQuery; -// -// Supports the following syntax -// -// jdbc:es://[host|ip] -// jdbc:es://[host|ip]:port/(prefix) -// jdbc:es://[host|ip]:port/(prefix)(?options=value&) -// -// Additional properties can be specified either through the Properties object or in the URL. In case of duplicates, the URL wins. -// - +/** + / Supports the following syntax + / + / jdbc:es://[host|ip] + / jdbc:es://[host|ip]:port/(prefix) + / jdbc:es://[host|ip]:port/(prefix)(?options=value&) + / + / Additional properties can be specified either through the Properties object or in the URL. In case of duplicates, the URL wins. + */ //TODO: beef this up for Security/SSL public class JdbcConfiguration extends ConnectionConfiguration { static final String URL_PREFIX = "jdbc:es://"; diff --git a/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConnection.java b/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConnection.java index 233211fcfde..17f8973cea3 100644 --- a/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConnection.java +++ b/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConnection.java @@ -332,14 +332,12 @@ public class JdbcConnection implements Connection, JdbcWrapper { @Override public void setClientInfo(String name, String value) throws SQLClientInfoException { checkOpenClientInfo(); - // no-op throw new SQLClientInfoException("Unsupported operation", null); } @Override public void setClientInfo(Properties properties) throws SQLClientInfoException { checkOpenClientInfo(); - // no-op throw new SQLClientInfoException("Unsupported operation", null); } diff --git a/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java b/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java index 6a23138b5dd..6b582a8f271 100644 --- a/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java +++ b/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java @@ -81,7 +81,8 @@ class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper { @Override public boolean nullsAreSortedAtEnd() throws SQLException { - return false; + // missing/null values are sorted (by default) last + return true; } @Override diff --git a/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/EsRelation.java b/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/EsRelation.java index 552e25eef48..73a95385446 100644 --- a/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/EsRelation.java +++ b/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/EsRelation.java @@ -11,9 +11,10 @@ import org.elasticsearch.xpack.sql.expression.FieldAttribute; import org.elasticsearch.xpack.sql.tree.Location; import org.elasticsearch.xpack.sql.tree.NodeInfo; import org.elasticsearch.xpack.sql.type.EsField; -import org.elasticsearch.xpack.sql.util.StringUtils; import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -91,8 +92,36 @@ public class EsRelation extends LeafPlan { return Objects.equals(index, other.index); } + private static final int TO_STRING_LIMIT = 52; + + private static String limitedToString(Collection c) { + Iterator it = c.iterator(); + if (!it.hasNext()) { + return "[]"; + } + + // ..] + StringBuilder sb = new StringBuilder(TO_STRING_LIMIT + 4); + sb.append('['); + for (;;) { + E e = it.next(); + String next = e == c ? "(this Collection)" : String.valueOf(e); + if (next.length() + sb.length() > TO_STRING_LIMIT) { + sb.append(next.substring(0, Math.max(0, TO_STRING_LIMIT - sb.length()))); + sb.append('.').append('.').append(']'); + return sb.toString(); + } else { + sb.append(next); + } + if (!it.hasNext()) { + return sb.append(']').toString(); + } + sb.append(',').append(' '); + } + } + @Override public String nodeString() { - return nodeName() + "[" + index + "]" + StringUtils.limitedToString(attrs); + return nodeName() + "[" + index + "]" + limitedToString(attrs); } -} +} \ No newline at end of file diff --git a/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/StringUtils.java b/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/StringUtils.java index ef4b76ca40f..b11864c8c8b 100644 --- a/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/StringUtils.java +++ b/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/StringUtils.java @@ -17,8 +17,6 @@ import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; -import java.util.Iterator; import java.util.List; import java.util.Locale; @@ -29,40 +27,6 @@ public abstract class StringUtils { public static final String EMPTY = ""; public static final String NEW_LINE = "\n"; - private static final int TO_STRING_LIMIT = 52; - - public static String limitedToString(Object o) { - String s = String.valueOf(o); - return s.length() > TO_STRING_LIMIT ? s.substring(0, TO_STRING_LIMIT).concat("...") : s; - } - - public static String limitedToString(Collection c) { - Iterator it = c.iterator(); - if (!it.hasNext()) { - return "[]"; - } - - // ..] - StringBuilder sb = new StringBuilder(TO_STRING_LIMIT + 4); - sb.append('['); - for (;;) { - E e = it.next(); - String next = e == c ? "(this Collection)" : String.valueOf(e); - if (next.length() + sb.length() > TO_STRING_LIMIT) { - sb.append(next.substring(0, Math.max(0, TO_STRING_LIMIT - sb.length()))); - sb.append('.').append('.').append(']'); - return sb.toString(); - } - else { - sb.append(next); - } - if (!it.hasNext()) { - return sb.append(']').toString(); - } - sb.append(',').append(' '); - } - } - //CamelCase to camel_case public static String camelCaseToUnderscore(String string) { if (!Strings.hasText(string)) {