Skip to content

Commit 42c57d3

Browse files
committed
SQL: Fix catalog filtering in SYS COLUMNS (elastic#40583)
Properly treat '%' as a wildcard for catalog filtering instead of doing a straight string match. Table filtering now considers aliases as well. Add escaping char for LIKE queries with user defined params Fix monotony of ORDINAL_POSITION Add integration test for SYS COLUMNS - currently running only inside single_node since the cluster name is test dependent. Add pattern unescaping for index names Fix elastic#40582 (cherry picked from commit 8e61b77)
1 parent 85fdab7 commit 42c57d3

File tree

13 files changed

+407
-233
lines changed

13 files changed

+407
-233
lines changed

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaData.java

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
*/
3434
class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper {
3535

36+
private static final String WILDCARD = "%";
37+
3638
private final JdbcConnection con;
3739

3840
JdbcDatabaseMetaData(JdbcConnection con) {
@@ -714,19 +716,19 @@ private boolean isDefaultCatalog(String catalog) throws SQLException {
714716
// null means catalog info is irrelevant
715717
// % means return all catalogs
716718
// EMPTY means return those without a catalog
717-
return catalog == null || catalog.equals(EMPTY) || catalog.equals("%") || catalog.equals(defaultCatalog());
719+
return catalog == null || catalog.equals(EMPTY) || catalog.equals(WILDCARD) || catalog.equals(defaultCatalog());
718720
}
719721

720722
private boolean isDefaultSchema(String schema) {
721723
// null means schema info is irrelevant
722724
// % means return all schemas`
723725
// EMPTY means return those without a schema
724-
return schema == null || schema.equals(EMPTY) || schema.equals("%");
726+
return schema == null || schema.equals(EMPTY) || schema.equals(WILDCARD);
725727
}
726728

727729
@Override
728730
public ResultSet getTables(String catalog, String schemaPattern, String tableNamePattern, String[] types) throws SQLException {
729-
String statement = "SYS TABLES CATALOG LIKE ? LIKE ?";
731+
String statement = "SYS TABLES CATALOG LIKE ? ESCAPE '\\' LIKE ? ESCAPE '\\' ";
730732

731733
if (types != null && types.length > 0) {
732734
statement += " TYPE ?";
@@ -739,8 +741,8 @@ public ResultSet getTables(String catalog, String schemaPattern, String tableNam
739741
}
740742

741743
PreparedStatement ps = con.prepareStatement(statement);
742-
ps.setString(1, catalog != null ? catalog.trim() : "%");
743-
ps.setString(2, tableNamePattern != null ? tableNamePattern.trim() : "%");
744+
ps.setString(1, catalog != null ? catalog.trim() : WILDCARD);
745+
ps.setString(2, tableNamePattern != null ? tableNamePattern.trim() : WILDCARD);
744746

745747
if (types != null && types.length > 0) {
746748
for (int i = 0; i < types.length; i++) {
@@ -785,14 +787,15 @@ public ResultSet getTableTypes() throws SQLException {
785787
return memorySet(con.cfg, columnInfo("TABLE_TYPES", "TABLE_TYPE"), data);
786788
}
787789

790+
788791
@Override
789792
public ResultSet getColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern)
790793
throws SQLException {
791-
PreparedStatement ps = con.prepareStatement("SYS COLUMNS CATALOG ? TABLE LIKE ? LIKE ?");
792-
// TODO: until passing null works, pass an empty string
793-
ps.setString(1, catalog != null ? catalog.trim() : EMPTY);
794-
ps.setString(2, tableNamePattern != null ? tableNamePattern.trim() : "%");
795-
ps.setString(3, columnNamePattern != null ? columnNamePattern.trim() : "%");
794+
PreparedStatement ps = con.prepareStatement("SYS COLUMNS CATALOG ? TABLE LIKE ? ESCAPE '\\' LIKE ? ESCAPE '\\'");
795+
// NB: catalog is not a pattern hence why null is send instead
796+
ps.setString(1, catalog != null ? catalog.trim() : null);
797+
ps.setString(2, tableNamePattern != null ? tableNamePattern.trim() : WILDCARD);
798+
ps.setString(3, columnNamePattern != null ? columnNamePattern.trim() : WILDCARD);
796799
return ps.executeQuery();
797800
}
798801

x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/JdbcSecurityIT.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -232,13 +232,13 @@ public void expectUnknownColumn(String user, String sql, String column) throws E
232232
public void checkNoMonitorMain(String user) throws Exception {
233233
// Without monitor/main the JDBC driver - ES server version comparison doesn't take place, which fails everything else
234234
expectUnauthorized("cluster:monitor/main", user, () -> es(userProperties(user)));
235-
expectUnauthorized("cluster:monitor/main", user, () -> es(userProperties(user)).getMetaData().getDatabaseMajorVersion());
235+
expectUnauthorized("cluster:monitor/main", user, () -> es(userProperties(user)).getMetaData().getDatabaseMajorVersion());
236236
expectUnauthorized("cluster:monitor/main", user, () -> es(userProperties(user)).getMetaData().getDatabaseMinorVersion());
237-
expectUnauthorized("cluster:monitor/main", user,
237+
expectUnauthorized("cluster:monitor/main", user,
238238
() -> es(userProperties(user)).createStatement().executeQuery("SELECT * FROM test"));
239-
expectUnauthorized("cluster:monitor/main", user,
239+
expectUnauthorized("cluster:monitor/main", user,
240240
() -> es(userProperties(user)).createStatement().executeQuery("SHOW TABLES LIKE 'test'"));
241-
expectUnauthorized("cluster:monitor/main", user,
241+
expectUnauthorized("cluster:monitor/main", user,
242242
() -> es(userProperties(user)).createStatement().executeQuery("DESCRIBE test"));
243243
}
244244

@@ -292,7 +292,7 @@ public void testMetaDataGetColumnsWorksAsFullAccess() throws Exception {
292292
expectActionMatchesAdmin(
293293
con -> con.getMetaData().getColumns(null, "%", "%t", "%"),
294294
"full_access",
295-
con -> con.getMetaData().getColumns(null, "%", "%", "%"));
295+
con -> con.getMetaData().getColumns(null, "%", "%t", "%"));
296296
}
297297

298298
public void testMetaDataGetColumnsWithNoAccess() throws Exception {

x-pack/plugin/sql/qa/single-node/src/test/java/org/elasticsearch/xpack/sql/qa/single_node/JdbcCsvSpecIT.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,26 @@
55
*/
66
package org.elasticsearch.xpack.sql.qa.single_node;
77

8+
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
9+
810
import org.elasticsearch.xpack.sql.qa.jdbc.CsvSpecTestCase;
911
import org.elasticsearch.xpack.sql.qa.jdbc.CsvTestUtils.CsvTestCase;
1012

13+
import java.util.ArrayList;
14+
import java.util.List;
15+
16+
import static org.elasticsearch.xpack.sql.qa.jdbc.CsvTestUtils.specParser;
17+
1118
public class JdbcCsvSpecIT extends CsvSpecTestCase {
19+
20+
21+
@ParametersFactory(argumentFormatting = PARAM_FORMATTING)
22+
public static List<Object[]> readScriptSpec() throws Exception {
23+
List<Object[]> list = new ArrayList<>();
24+
list.addAll(CsvSpecTestCase.readScriptSpec());
25+
return readScriptSpec("/single-node-only/command-sys.csv-spec", specParser());
26+
}
27+
1228
public JdbcCsvSpecIT(String fileName, String groupName, String testName, Integer lineNumber, CsvTestCase testCase) {
1329
super(fileName, groupName, testName, lineNumber, testCase);
1430
}

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/CsvTestUtils.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ private static String resolveColumnType(String type) {
155155
return "timestamp";
156156
case "bt":
157157
return "byte";
158+
case "sh":
159+
return "short";
158160
default:
159161
return type;
160162
}

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/SpecBaseIntegrationTestCase.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.io.InputStream;
2020
import java.io.InputStreamReader;
2121
import java.net.URL;
22+
import java.net.URLConnection;
2223
import java.nio.charset.StandardCharsets;
2324
import java.sql.Connection;
2425
import java.sql.ResultSet;
@@ -195,6 +196,9 @@ public interface Parser {
195196

196197
@SuppressForbidden(reason = "test reads from jar")
197198
public static InputStream readFromJarUrl(URL source) throws IOException {
198-
return source.openStream();
199+
URLConnection con = source.openConnection();
200+
// do not to cache files (to avoid keeping file handles around)
201+
con.setUseCaches(false);
202+
return con.getInputStream();
199203
}
200204
}

x-pack/plugin/sql/qa/src/main/resources/setup_mock_metadata_get_columns.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ FROM DUAL
3030
UNION ALL
3131
SELECT null, 'test1', 'name.keyword', 12, 'KEYWORD', 0, 2147483647, null, null,
3232
1, -- columnNullable
33-
null, null, 12, 0, 2147483647, 1, 'YES', null, null, null, null, 'NO', 'NO'
33+
null, null, 12, 0, 2147483647, 2, 'YES', null, null, null, null, 'NO', 'NO'
3434
FROM DUAL
3535
UNION ALL
3636
SELECT null, 'test2', 'date', 93, 'DATETIME', 29, 8, null, null,

0 commit comments

Comments
 (0)