From 202c716939bdc6c074113350de50e7bd8fd9dd95 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Mon, 8 Apr 2019 16:45:25 +0300 Subject: [PATCH] SQL: Prefer resultSets over exceptions in metadata (#40641) Changed the JDBC metadata to return empty results sets instead of throwing SQLFeatureNotSupported as it seems a more safer/compatible approach for consumers. Fix #40533 (cherry picked from commit ef2d2527c2b5140556fd477e7ff6ea36966684da) --- .../xpack/sql/jdbc/JdbcConfiguration.java | 7 +- .../xpack/sql/jdbc/JdbcConnection.java | 7 +- .../xpack/sql/jdbc/JdbcDatabaseMetaData.java | 165 +++++++++++++++--- .../xpack/sql/jdbc/JdbcHttpClient.java | 15 +- .../xpack/sql/jdbc/JdbcResultSet.java | 3 +- .../sql/jdbc/JdbcDatabaseMetaDataTests.java | 108 +++++++++++- .../sql/client/ConnectionConfiguration.java | 8 +- 7 files changed, 272 insertions(+), 41 deletions(-) diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfiguration.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfiguration.java index 36df3488ff8bd..7a9154c10ac4e 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfiguration.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfiguration.java @@ -162,7 +162,7 @@ private JdbcConfiguration(URI baseURI, String u, Properties props) throws JdbcSQ } @Override - protected Collection extraOptions() { + protected Collection extraOptions() { return OPTION_NAMES; } @@ -192,9 +192,8 @@ public static boolean canAccept(String url) { public DriverPropertyInfo[] driverPropertyInfo() { List info = new ArrayList<>(); - for (String option : OPTION_NAMES) { - String value = null; - DriverPropertyInfo prop = new DriverPropertyInfo(option, value); + for (String option : optionNames()) { + DriverPropertyInfo prop = new DriverPropertyInfo(option, null); info.add(prop); } diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcConnection.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcConnection.java index c682c5ac05c63..09096fbe405a0 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcConnection.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcConnection.java @@ -45,9 +45,12 @@ class JdbcConnection implements Connection, JdbcWrapper { * If we remove it, we need to make sure no other types of Exceptions (runtime or otherwise) are thrown */ JdbcConnection(JdbcConfiguration connectionInfo) throws SQLException { - cfg = connectionInfo; - client = new JdbcHttpClient(connectionInfo); + this(connectionInfo, true); + } + JdbcConnection(JdbcConfiguration connectionInfo, boolean checkServer) throws SQLException { + cfg = connectionInfo; + client = new JdbcHttpClient(connectionInfo, checkServer); url = connectionInfo.connectionString(); userName = connectionInfo.authUser(); } diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaData.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaData.java index 5697453730455..d2f980542b86c 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaData.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaData.java @@ -10,15 +10,17 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; +import java.sql.DriverPropertyInfo; import java.sql.JDBCType; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.RowIdLifetime; import java.sql.SQLException; -import java.sql.SQLFeatureNotSupportedException; import java.util.ArrayList; import java.util.List; +import static java.sql.JDBCType.BIGINT; +import static java.sql.JDBCType.BOOLEAN; import static java.sql.JDBCType.INTEGER; import static java.sql.JDBCType.SMALLINT; import static org.elasticsearch.xpack.sql.client.StringUtils.EMPTY; @@ -664,8 +666,7 @@ public boolean dataDefinitionIgnoredInTransactions() throws SQLException { // https://www.postgresql.org/docs/9.0/static/infoschema-routines.html @Override public ResultSet getProcedures(String catalog, String schemaPattern, String procedureNamePattern) throws SQLException { - return emptySet(con.cfg, - "ROUTINES", + return emptySet(con.cfg, "ROUTINES", "PROCEDURE_CAT", "PROCEDURE_SCHEM", "PROCEDURE_NAME", @@ -680,8 +681,7 @@ public ResultSet getProcedures(String catalog, String schemaPattern, String proc @Override public ResultSet getProcedureColumns(String catalog, String schemaPattern, String procedureNamePattern, String columnNamePattern) throws SQLException { - return emptySet(con.cfg, - "PARAMETERS", + return emptySet(con.cfg, "ROUTINES_COLUMNS", "PROCEDURE_CAT", "PROCEDURE_SCHEM", "PROCEDURE_NAME", @@ -775,14 +775,14 @@ public ResultSet getSchemas(String catalog, String schemaPattern) throws SQLExce public ResultSet getCatalogs() throws SQLException { // TABLE_CAT is the first column Object[][] data = queryColumn(con, "SYS TABLES CATALOG LIKE '%'", 1); - return memorySet(con.cfg, columnInfo("", "TABLE_CAT"), data); + return memorySet(con.cfg, columnInfo("CATALOGS", "TABLE_CAT"), data); } @Override public ResultSet getTableTypes() throws SQLException { // TABLE_TYPE (4) Object[][] data = queryColumn(con, "SYS TABLES TYPE '%'", 4); - return memorySet(con.cfg, columnInfo("", "TABLE_TYPE"), data); + return memorySet(con.cfg, columnInfo("TABLE_TYPES", "TABLE_TYPE"), data); } @Override @@ -798,43 +798,128 @@ public ResultSet getColumns(String catalog, String schemaPattern, String tableNa @Override public ResultSet getColumnPrivileges(String catalog, String schema, String table, String columnNamePattern) throws SQLException { - throw new SQLFeatureNotSupportedException("Privileges not supported"); + return emptySet(con.cfg, "", + "TABLE_CAT", + "TABLE_SCHEM", + "TABLE_NAME", + "COLUMN_NAME", + "GRANTOR", + "GRANTEE", + "PRIVILEGE", + "IS_GRANTABLE"); } @Override public ResultSet getTablePrivileges(String catalog, String schemaPattern, String tableNamePattern) throws SQLException { - throw new SQLFeatureNotSupportedException("Privileges not supported"); + return emptySet(con.cfg, "", + "TABLE_CAT", + "TABLE_SCHEM", + "TABLE_NAME", + "GRANTOR", + "GRANTEE", + "PRIVILEGE", + "IS_GRANTABLE"); } @Override public ResultSet getBestRowIdentifier(String catalog, String schema, String table, int scope, boolean nullable) throws SQLException { - throw new SQLFeatureNotSupportedException("Row identifiers not supported"); + return emptySet(con.cfg, "", + "SCOPE", SMALLINT, + "COLUMN_NAME", + "DATA_TYPE", INTEGER, + "TYPE_NAME", + "COLUMN_SIZE", INTEGER, + "BUFFER_LENGTH", INTEGER, + "DECIMAL_DIGITS", SMALLINT, + "PSEUDO_COLUMN", SMALLINT); } @Override public ResultSet getVersionColumns(String catalog, String schema, String table) throws SQLException { - throw new SQLFeatureNotSupportedException("Version column not supported yet"); + return emptySet(con.cfg, "", + "SCOPE", SMALLINT, + "COLUMN_NAME", + "DATA_TYPE", INTEGER, + "TYPE_NAME", + "COLUMN_SIZE", INTEGER, + "BUFFER_LENGTH", INTEGER, + "DECIMAL_DIGITS", SMALLINT, + "PSEUDO_COLUMN", SMALLINT); } @Override public ResultSet getPrimaryKeys(String catalog, String schema, String table) throws SQLException { - throw new SQLFeatureNotSupportedException("Primary keys not supported"); + return emptySet(con.cfg, "", + "TABLE_CAT", + "TABLE_SCHEM", + "TABLE_NAME", + "COLUMN_NAME", + "KEY_SEQ", SMALLINT, + "PK_NAME"); } @Override public ResultSet getImportedKeys(String catalog, String schema, String table) throws SQLException { - throw new SQLFeatureNotSupportedException("Imported keys not supported"); + return emptySet(con.cfg, "", + "PKTABLE_CAT", + "PKTABLE_SCHEM", + "PKTABLE_NAME", + "PKCOLUMN_NAME", + "FKTABLE_CAT", + "FKTABLE_SCHEM", + "FKTABLE_NAME", + "FKCOLUMN_NAME", + "KEY_SEQ", SMALLINT, + "UPDATE_RULE ", SMALLINT, + "DELETE_RULE ", SMALLINT, + "FK_NAME", + "PK_NAME ", + "DEFERRABILITY", SMALLINT, + "IS_NULLABLE" + ); } @Override public ResultSet getExportedKeys(String catalog, String schema, String table) throws SQLException { - throw new SQLFeatureNotSupportedException("Exported keys not supported"); + return emptySet(con.cfg, "", + "PKTABLE_CAT", + "PKTABLE_SCHEM", + "PKTABLE_NAME", + "PKCOLUMN_NAME", + "FKTABLE_CAT", + "FKTABLE_SCHEM", + "FKTABLE_NAME", + "FKCOLUMN_NAME", + "KEY_SEQ", SMALLINT, + "UPDATE_RULE ", SMALLINT, + "DELETE_RULE ", SMALLINT, + "FK_NAME", + "PK_NAME ", + "DEFERRABILITY", SMALLINT, + "IS_NULLABLE" + ); } @Override public ResultSet getCrossReference(String parentCatalog, String parentSchema, String parentTable, String foreignCatalog, String foreignSchema, String foreignTable) throws SQLException { - throw new SQLFeatureNotSupportedException("Cross reference not supported"); + return emptySet(con.cfg, "", + "PKTABLE_CAT", + "PKTABLE_SCHEM", + "PKTABLE_NAME", + "PKCOLUMN_NAME", + "FKTABLE_CAT", + "FKTABLE_SCHEM", + "FKTABLE_NAME", + "FKCOLUMN_NAME", + "KEY_SEQ", SMALLINT, + "UPDATE_RULE ", SMALLINT, + "DELETE_RULE ", SMALLINT, + "FK_NAME", + "PK_NAME ", + "DEFERRABILITY", SMALLINT, + "IS_NULLABLE" + ); } @Override @@ -844,7 +929,22 @@ public ResultSet getTypeInfo() throws SQLException { @Override public ResultSet getIndexInfo(String catalog, String schema, String table, boolean unique, boolean approximate) throws SQLException { - throw new SQLFeatureNotSupportedException("Indicies not supported"); + return emptySet(con.cfg, "", + "TABLE_CAT", + "TABLE_SCHEM", + "TABLE_NAME", + "NON_UNIQUE", BOOLEAN, + "INDEX_QUALIFIER", + "INDEX_NAME", + "TYPE", SMALLINT, + "ORDINAL_POSITION", SMALLINT, + "COLUMN_NAME", + "ASC_OR_DESC", + "CARDINALITY", BIGINT, + "PAGES", BIGINT, + "FILTER_CONDITION", + "TYPE_NAME" + ); } @Override @@ -909,7 +1009,7 @@ public boolean supportsBatchUpdates() throws SQLException { @Override public ResultSet getUDTs(String catalog, String schemaPattern, String typeNamePattern, int[] types) throws SQLException { - return emptySet(con.cfg, + return emptySet(con.cfg, "", "USER_DEFINED_TYPES", "TYPE_CAT", "TYPE_SCHEM", @@ -947,7 +1047,7 @@ public boolean supportsGetGeneratedKeys() throws SQLException { @Override public ResultSet getSuperTypes(String catalog, String schemaPattern, String typeNamePattern) throws SQLException { - return emptySet(con.cfg, + return emptySet(con.cfg, "", "SUPER_TYPES", "TYPE_CAT", "TYPE_SCHEM", @@ -960,7 +1060,7 @@ public ResultSet getSuperTypes(String catalog, String schemaPattern, String type @Override public ResultSet getSuperTables(String catalog, String schemaPattern, String tableNamePattern) throws SQLException { - return emptySet(con.cfg, "SUPER_TABLES", + return emptySet(con.cfg, "", "TABLE_CAT", "TABLE_SCHEM", "TABLE_NAME", @@ -970,7 +1070,7 @@ public ResultSet getSuperTables(String catalog, String schemaPattern, String tab @Override public ResultSet getAttributes(String catalog, String schemaPattern, String typeNamePattern, String attributeNamePattern) throws SQLException { - return emptySet(con.cfg, + return emptySet(con.cfg, "", "ATTRIBUTES", "TYPE_CAT", "TYPE_SCHEM", @@ -1057,12 +1157,27 @@ public boolean autoCommitFailureClosesAllResultSets() throws SQLException { @Override public ResultSet getClientInfoProperties() throws SQLException { - throw new SQLException("Client info not implemented yet"); + DriverPropertyInfo[] info = con.cfg.driverPropertyInfo(); + Object[][] data = new Object[info.length][]; + + for (int i = 0; i < data.length; i++) { + data[i] = new Object[4]; + data[i][0] = info[i].name; + data[i][1] = Integer.valueOf(-1); + data[i][2] = EMPTY; + data[i][3] = EMPTY; + } + + return memorySet(con.cfg, columnInfo("", + "NAME", + "MAX_LEN", INTEGER, + "DEFAULT_VALUE", + "DESCRIPTION"), data); } @Override public ResultSet getFunctions(String catalog, String schemaPattern, String functionNamePattern) throws SQLException { - return emptySet(con.cfg, + return emptySet(con.cfg, "", "FUNCTIONS", "FUNCTION_CAT", "FUNCTION_SCHEM", @@ -1075,7 +1190,7 @@ public ResultSet getFunctions(String catalog, String schemaPattern, String funct @Override public ResultSet getFunctionColumns(String catalog, String schemaPattern, String functionNamePattern, String columnNamePattern) throws SQLException { - return emptySet(con.cfg, + return emptySet(con.cfg, "", "FUNCTION_COLUMNS", "FUNCTION_CAT", "FUNCTION_SCHEM", @@ -1098,7 +1213,7 @@ public ResultSet getFunctionColumns(String catalog, String schemaPattern, String @Override public ResultSet getPseudoColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern) throws SQLException { - return emptySet(con.cfg, + return emptySet(con.cfg, "", "PSEUDO_COLUMNS", "TABLE_CAT", "TABLE_SCHEM", @@ -1213,7 +1328,7 @@ public Object column(int column) { @Override public int batchSize() { - return data.length; + return ObjectUtils.isEmpty(data) ? 0 : data.length; } @Override diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java index 853993d56f416..5954d2b6c636f 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java @@ -30,17 +30,23 @@ class JdbcHttpClient { private final HttpClient httpClient; private final JdbcConfiguration conCfg; - private final InfoResponse serverInfo; + private InfoResponse serverInfo; /** * The SQLException is the only type of Exception the JDBC API can throw (and that the user expects). * If we remove it, we need to make sure no other types of Exceptions (runtime or otherwise) are thrown */ JdbcHttpClient(JdbcConfiguration conCfg) throws SQLException { + this(conCfg, true); + } + + JdbcHttpClient(JdbcConfiguration conCfg, boolean checkServer) throws SQLException { httpClient = new HttpClient(conCfg); this.conCfg = conCfg; - this.serverInfo = fetchServerInfo(); - checkServerVersion(); + if (checkServer) { + this.serverInfo = fetchServerInfo(); + checkServerVersion(); + } } boolean ping(long timeoutInMs) throws SQLException { @@ -77,6 +83,9 @@ boolean queryClose(String cursor) throws SQLException { } InfoResponse serverInfo() throws SQLException { + if (serverInfo == null) { + serverInfo = fetchServerInfo(); + } return serverInfo; } diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java index 9b1fcb48901a7..14e8340a54424 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java @@ -1203,6 +1203,7 @@ public void updateNClob(String columnLabel, Reader reader) throws SQLException { @Override public String toString() { - return format(Locale.ROOT, "%s:row %d", getClass().getSimpleName(), rowNumber); + return format(Locale.ROOT, "%s:row %d:cursor size %d:%s", getClass().getSimpleName(), rowNumber, cursor.batchSize(), + cursor.columns()); } } diff --git a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaDataTests.java b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaDataTests.java index e24deaced9dd7..065807117ea4b 100644 --- a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaDataTests.java +++ b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaDataTests.java @@ -6,15 +6,119 @@ package org.elasticsearch.xpack.sql.jdbc; +import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.test.ESTestCase; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.Properties; + public class JdbcDatabaseMetaDataTests extends ESTestCase { - private JdbcDatabaseMetaData md = new JdbcDatabaseMetaData(null); + private JdbcDatabaseMetaData md = null; + + { + try { + md = new JdbcDatabaseMetaData( + new JdbcConnection(JdbcConfiguration.create("jdbc:es://localhost:9200/", new Properties(), 10), false)); + } catch (Exception ex) { + throw new RuntimeException(ex); + } + } public void testSeparators() throws Exception { assertEquals(":", md.getCatalogSeparator()); assertEquals("\"", md.getIdentifierQuoteString()); assertEquals("\\", md.getSearchStringEscape()); } -} + + public void testGetProcedures() throws Exception { + testEmptySet(() -> md.getProcedures(null, null, null)); + } + + public void testGetProcedureColumns() throws Exception { + testEmptySet(() -> md.getProcedureColumns(null, null, null, null)); + } + + public void testGetColumnPrivileges() throws Exception { + testEmptySet(() -> md.getColumnPrivileges(null, null, null, null)); + } + + public void testGetTablePrivileges() throws Exception { + testEmptySet(() -> md.getTablePrivileges(null, null, null)); + } + + public void testGetBestRowIdentifier() throws Exception { + testEmptySet(() -> md.getBestRowIdentifier(null, null, null, 0, false)); + } + + public void testGetVersionColumns() throws Exception { + testEmptySet(() -> md.getVersionColumns(null, null, null)); + } + + public void testGetPrimaryKeys() throws Exception { + testEmptySet(() -> md.getPrimaryKeys(null, null, null)); + } + + public void testGetImportedKeys() throws Exception { + testEmptySet(() -> md.getImportedKeys(null, null, null)); + } + + public void testGetExportedKeys() throws Exception { + testEmptySet(() -> md.getExportedKeys(null, null, null)); + } + + public void testGetCrossReference() throws Exception { + testEmptySet(() -> md.getCrossReference(null, null, null, null, null, null)); + } + + public void testGetIndexInfo() throws Exception { + testEmptySet(() -> md.getIndexInfo(null, null, null, false, false)); + } + + public void testGetUDTs() throws Exception { + testEmptySet(() -> md.getUDTs(null, null, null, null)); + } + + public void testGetSuperTypes() throws Exception { + testEmptySet(() -> md.getSuperTypes(null, null, null)); + } + + public void testGetSuperTables() throws Exception { + testEmptySet(() -> md.getSuperTables(null, null, null)); + } + + public void testGetAttributes() throws Exception { + testEmptySet(() -> md.getAttributes(null, null, null, null)); + } + + public void testGetFunctions() throws Exception { + testEmptySet(() -> md.getFunctions(null, null, null)); + } + + public void testGetFunctionColumns() throws Exception { + testEmptySet(() -> md.getFunctionColumns(null, null, null, null)); + } + + public void testGetPseudoColumns() throws Exception { + testEmptySet(() -> md.getPseudoColumns(null, null, null, null)); + } + + private static void testEmptySet(CheckedSupplier supplier) throws SQLException { + try (ResultSet result = supplier.get()) { + assertNotNull(result); + assertFalse(result.next()); + } + } + + public void testGetClientInfoProperties() throws Exception { + try (ResultSet result = md.getClientInfoProperties()) { + assertNotNull(result); + assertTrue(result.next()); + assertNotNull(result.getString(1)); + assertEquals(-1, result.getInt(2)); + assertEquals("", result.getString(3)); + assertEquals("", result.getString(4)); + } + } +} \ No newline at end of file diff --git a/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/ConnectionConfiguration.java b/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/ConnectionConfiguration.java index c3c89906c2302..591762b18a985 100644 --- a/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/ConnectionConfiguration.java +++ b/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/ConnectionConfiguration.java @@ -7,13 +7,13 @@ import java.net.URI; import java.net.URISyntaxException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Enumeration; import java.util.LinkedHashSet; import java.util.Properties; import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.TimeUnit; import java.util.function.Function; @@ -148,13 +148,13 @@ private static URI normalizeSchema(URI uri, String connectionString, boolean isS } } - private Collection optionNames() { - Collection options = new ArrayList<>(OPTION_NAMES); + protected Collection optionNames() { + Set options = new TreeSet<>(OPTION_NAMES); options.addAll(extraOptions()); return options; } - protected Collection extraOptions() { + protected Collection extraOptions() { return emptyList(); }