* [tarantool-patches] [PATCH] jdbc: fix primary keys meta retrieval
@ 2018-09-26 9:33 Sergei Kalashnikov
2018-10-01 13:04 ` [tarantool-patches] " Alexander Turenko
0 siblings, 1 reply; 11+ messages in thread
From: Sergei Kalashnikov @ 2018-09-26 9:33 UTC (permalink / raw)
To: tarantool-patches; +Cc: Sergei Kalashnikov
Fixed parsing primary keys metadata response.
Updated integration tests.
Closes #41
---
https://github.com/tarantool/tarantool-java/issues/41
https://github.com/ztarvos/tarantool-java/commits/gh-41-fix-jdbc-pk-meta
.../org/tarantool/jdbc/SQLDatabaseMetadata.java | 8 +++---
.../java/org/tarantool/jdbc/AbstractJdbcIT.java | 11 ++++----
.../org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java | 32 ++++++++++++++++++++--
3 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
index e43bcdc..790bbe7 100644
--- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
+++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
@@ -774,12 +774,12 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
List<Map<String, Object>> fields = (List<Map<String, Object>>) space.get(FORMAT_IDX);
List<List<Object>> indexes = (List<List<Object>>) connection.connection.select(_VINDEX, 0, Arrays.asList(space.get(SPACE_ID_IDX), 0), 0, 1, 0);
List<Object> primaryKey = indexes.get(0);
- List<List<Object>> parts = (List<List<Object>>) primaryKey.get(INDEX_FORMAT_IDX);
+ List<Map<String, Object>> parts = (List<Map<String, Object>>) primaryKey.get(INDEX_FORMAT_IDX);
for (int i = 0; i < parts.size(); i++) {
- List<Object> part = parts.get(i);
- int ordinal = ((Number) part.get(0)).intValue();
+ Map<String, Object> part = parts.get(i);
+ int ordinal = ((Number) part.get("field")).intValue();
String column = (String) fields.get(ordinal).get("name");
- rows.add(Arrays.asList(table, column, i + 1, "primary", primaryKey.get(NAME_IDX)));
+ rows.add(Arrays.asList(table, column, i + 1, primaryKey.get(NAME_IDX)));
}
}
return new SQLNullResultSet((JDBCBridge.mock(
diff --git a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
index 21e3f53..abed9b6 100644
--- a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
+++ b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
@@ -32,9 +32,6 @@ public abstract class AbstractJdbcIT {
private static String URL = String.format("tarantool://%s:%d?user=%s&password=%s", host, port, user, pass);
private static String[] initSql = new String[] {
- "DROP TABLE IF EXISTS test",
- "DROP TABLE IF EXISTS test_types",
-
"CREATE TABLE test(id INT PRIMARY KEY, val VARCHAR(100))",
"INSERT INTO test VALUES (1, 'one'), (2, 'two'), (3, 'three')",
@@ -78,12 +75,15 @@ public abstract class AbstractJdbcIT {
"X'010203040506'," + //LONGVARBINARY
"'1983-03-14'," + //DATE
"'12:01:06'," + //TIME
- "129479994)" //TIMESTAMP
+ "129479994)", //TIMESTAMP
+
+ "CREATE TABLE test_compound(id1 INT, id2 INT, val VARCHAR(100), PRIMARY KEY (id1, id2))"
};
private static String[] cleanSql = new String[] {
"DROP TABLE IF EXISTS test",
- "DROP TABLE IF EXISTS test_types"
+ "DROP TABLE IF EXISTS test_types",
+ "DROP TABLE IF EXISTS test_compound"
};
static Object[] testRow = new Object[] {
@@ -112,6 +112,7 @@ public abstract class AbstractJdbcIT {
@BeforeAll
public static void setupEnv() throws Exception {
+ sqlExec(cleanSql);
sqlExec(initSql);
}
diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
index b934bbe..984a60b 100644
--- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
+++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
@@ -1,6 +1,5 @@
package org.tarantool.jdbc;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.BeforeEach;
@@ -45,6 +44,9 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
assertTrue(rs.next());
assertEquals("TEST_TYPES", rs.getString("TABLE_NAME"));
+ assertTrue(rs.next());
+ assertEquals("TEST_COMPOUND", rs.getString("TABLE_NAME"));
+
assertFalse(rs.next());
rs.close();
@@ -84,7 +86,6 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
rs.close();
}
- @Disabled(value="Test ignored, issue#41")
@Test
public void testGetPrimaryKeys() throws SQLException {
ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
@@ -103,4 +104,31 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
rs.close();
}
+
+ @Test
+ public void testGetPrimaryKeysCompound() throws SQLException {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "TEST_COMPOUND");
+
+ assertNotNull(rs);
+ assertTrue(rs.next());
+
+ assertNull(rs.getString("TABLE_CAT"));
+ assertNull(rs.getString("TABLE_SCHEM"));
+ assertEquals("TEST_COMPOUND", rs.getString("TABLE_NAME"));
+ assertEquals("ID1", rs.getString("COLUMN_NAME"));
+ assertEquals(1, rs.getInt("KEY_SEQ"));
+ assertEquals("pk_unnamed_TEST_COMPOUND_1", rs.getString("PK_NAME"));
+
+ assertTrue(rs.next());
+ assertNull(rs.getString("TABLE_CAT"));
+ assertNull(rs.getString("TABLE_SCHEM"));
+ assertEquals("TEST_COMPOUND", rs.getString("TABLE_NAME"));
+ assertEquals("ID2", rs.getString("COLUMN_NAME"));
+ assertEquals(2, rs.getInt("KEY_SEQ"));
+ assertEquals("pk_unnamed_TEST_COMPOUND_1", rs.getString("PK_NAME"));
+
+ assertFalse(rs.next());
+
+ rs.close();
+ }
}
--
1.8.3.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-09-26 9:33 [tarantool-patches] [PATCH] jdbc: fix primary keys meta retrieval Sergei Kalashnikov
@ 2018-10-01 13:04 ` Alexander Turenko
2018-10-02 17:09 ` Sergei Kalashnikov
2018-10-09 9:21 ` Sergei Kalashnikov
0 siblings, 2 replies; 11+ messages in thread
From: Alexander Turenko @ 2018-10-01 13:04 UTC (permalink / raw)
To: Sergei Kalashnikov; +Cc: tarantool-patches
Hi!
Sorry for the late response.
From [1]:
> Retrieves a description of the given table's primary key columns. They are ordered by COLUMN_NAME.
That is strange, but that is. Maybe we should also check the standard.
[1]: https://docs.oracle.com/javase/7/docs/api/java/sql/DatabaseMetaData.html#getPrimaryKeys(java.lang.String,%20java.lang.String,%20java.lang.String)
Tests are ok for me.
Maybe we also should set some defined behaviour for the case when
'catalog' and 'schema' are given for getPrimaryKeys. I think we should
ignore them and return nulls in TABLE_CAT and TABLE_SCHEM. I think we
should have corresponding test case.
More comment are inline.
WBR, Alexander Turenko.
On Wed, Sep 26, 2018 at 12:33:28PM +0300, Sergei Kalashnikov wrote:
> Fixed parsing primary keys metadata response.
> Updated integration tests.
>
> Closes #41
> ---
> https://github.com/tarantool/tarantool-java/issues/41
> https://github.com/ztarvos/tarantool-java/commits/gh-41-fix-jdbc-pk-meta
>
> .../org/tarantool/jdbc/SQLDatabaseMetadata.java | 8 +++---
> .../java/org/tarantool/jdbc/AbstractJdbcIT.java | 11 ++++----
> .../org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java | 32 ++++++++++++++++++++--
> 3 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
> index e43bcdc..790bbe7 100644
> --- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
> +++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
> @@ -774,12 +774,12 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
> List<Map<String, Object>> fields = (List<Map<String, Object>>) space.get(FORMAT_IDX);
> List<List<Object>> indexes = (List<List<Object>>) connection.connection.select(_VINDEX, 0, Arrays.asList(space.get(SPACE_ID_IDX), 0), 0, 1, 0);
> List<Object> primaryKey = indexes.get(0);
> - List<List<Object>> parts = (List<List<Object>>) primaryKey.get(INDEX_FORMAT_IDX);
> + List<Map<String, Object>> parts = (List<Map<String, Object>>) primaryKey.get(INDEX_FORMAT_IDX);
I think we should check the type to give a proper error message in the
case when a user give 'native' tarantool space in parameters.
Corresponding test case should be added.
Don't sure how it will be wrapped in Java, I just about the following:
6th field of _vspace tuple for a 'native' space:
[[0, 'unsigned'], [1, 'unsigned']]
(Don't sure whether it is list or map in the binary protocol; likely
map.)
6th field of _vspace tuple for a sql space:
[{'sort_order': 'asc', 'type': 'scalar', 'field': 0,
'nullable_action': 'abort', 'is_nullable': false}]
What would going on if a user give us 'native' space name?
> for (int i = 0; i < parts.size(); i++) {
> - List<Object> part = parts.get(i);
> - int ordinal = ((Number) part.get(0)).intValue();
> + Map<String, Object> part = parts.get(i);
> + int ordinal = ((Number) part.get("field")).intValue();
> String column = (String) fields.get(ordinal).get("name");
Just note: it is not mandatory to space format fields to have a name,
but SQL spaces have them. Maybe a comment needed here.
> - rows.add(Arrays.asList(table, column, i + 1, "primary", primaryKey.get(NAME_IDX)));
> + rows.add(Arrays.asList(table, column, i + 1, primaryKey.get(NAME_IDX)));
> }
> }
> return new SQLNullResultSet((JDBCBridge.mock(
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-10-01 13:04 ` [tarantool-patches] " Alexander Turenko
@ 2018-10-02 17:09 ` Sergei Kalashnikov
2018-10-09 9:21 ` Sergei Kalashnikov
1 sibling, 0 replies; 11+ messages in thread
From: Sergei Kalashnikov @ 2018-10-02 17:09 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Hi Alexander!
Thanks for the review. Please see my replies inline below.
On Mon, Oct 01, 2018 at 04:04:27PM +0300, Alexander Turenko wrote:
> Hi!
>
> Sorry for the late response.
>
> From [1]:
>
> > Retrieves a description of the given table's primary key columns. They are ordered by COLUMN_NAME.
>
> That is strange, but that is. Maybe we should also check the standard.
>
> [1]: https://docs.oracle.com/javase/7/docs/api/java/sql/DatabaseMetaData.html#getPrimaryKeys(java.lang.String,%20java.lang.String,%20java.lang.String)
The standard lacks such details. It relies on the underlying Java API docs.
And some vendors apply different ordering (e.g. table_name, pk_name, key_seq).
I added the sorting in revised patch (please note how the definition of
'test_compound' table has changed in tests).
I have noticed another deviation from specification. The order in which columns are
returned in the resultset is not correct. The TABLE_CAT and TABLE_SCHEM must go first.
I have fixed this as well. Plz see the new test 'testGetPrimaryKeysOrderOfColumns'.
>
> Tests are ok for me.
>
> Maybe we also should set some defined behaviour for the case when
> 'catalog' and 'schema' are given for getPrimaryKeys. I think we should
> ignore them and return nulls in TABLE_CAT and TABLE_SCHEM. I think we
> should have corresponding test case.
I agree. I see that, for instance, postgreSQL JDBC are doing that for catalog.
I added a test 'testGetPrimaryKeysIgnoresCatalogSchema' for that.
> >
> > diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
> > index e43bcdc..790bbe7 100644
> > --- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
> > +++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
> > @@ -774,12 +774,12 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
> > List<Map<String, Object>> fields = (List<Map<String, Object>>) space.get(FORMAT_IDX);
> > List<List<Object>> indexes = (List<List<Object>>) connection.connection.select(_VINDEX, 0, Arrays.asList(space.get(SPACE_ID_IDX), 0), 0, 1, 0);
> > List<Object> primaryKey = indexes.get(0);
> > - List<List<Object>> parts = (List<List<Object>>) primaryKey.get(INDEX_FORMAT_IDX);
> > + List<Map<String, Object>> parts = (List<Map<String, Object>>) primaryKey.get(INDEX_FORMAT_IDX);
>
> I think we should check the type to give a proper error message in the
> case when a user give 'native' tarantool space in parameters.
> Corresponding test case should be added.
>
> Don't sure how it will be wrapped in Java, I just about the following:
>
> 6th field of _vspace tuple for a 'native' space:
>
> [[0, 'unsigned'], [1, 'unsigned']]
>
> (Don't sure whether it is list or map in the binary protocol; likely
> map.)
>
> 6th field of _vspace tuple for a sql space:
>
> [{'sort_order': 'asc', 'type': 'scalar', 'field': 0,
> 'nullable_action': 'abort', 'is_nullable': false}]
>
> What would going on if a user give us 'native' space name?
I've made some tests. For the 6th field of '_vspace' tuple, I've got the same 'List<Map>'
in both native and sql space cases, the difference was only in map contents.
However for the 5th field of '_vindex' tuple, it is 'List<List>' for the native space
and 'List<Map>' for the sql space.
So, I added extensive type checking and throw an exception in case unexpected type is encountered.
See tests 'testGetPrimaryKeyNonSQLSpace' and 'testDatabaseMetaDataGetPrimaryKeysFormatError'.
Are you certain that we should return the error in case we have been given a native space?
I went for empty resultset instead. Hope you are OK with that.
>
> > for (int i = 0; i < parts.size(); i++) {
> > - List<Object> part = parts.get(i);
> > - int ordinal = ((Number) part.get(0)).intValue();
> > + Map<String, Object> part = parts.get(i);
> > + int ordinal = ((Number) part.get("field")).intValue();
> > String column = (String) fields.get(ordinal).get("name");
>
> Just note: it is not mandatory to space format fields to have a name,
> but SQL spaces have them. Maybe a comment needed here.
>
OK, done.
Please find below the fixed patch.
==================================================================
From 705c3ba3746cf5e196d9c58419348936817ea327 Mon Sep 17 00:00:00 2001
From: Sergei Kalashnikov <ztarvos@gmail.com>
Date: Tue, 2 Oct 2018 19:12:37 +0300
Subject: [PATCH] jdbc: fix primary keys meta retrieval
Fixed several mistakes in get primary keys metadata API.
Corrected type mismatch when parsing server response.
Added sorting of result rows by column name.
Fixed order of columns in result set.
Wrapped errors into SQLException.
Improved test coverage.
Closes #41
---
https://github.com/tarantool/tarantool-java/issues/41
https://github.com/ztarvos/tarantool-java/commits/gh-41-fix-jdbc-pk-meta
.../org/tarantool/jdbc/SQLDatabaseMetadata.java | 78 +++++++++++++----
.../java/org/tarantool/jdbc/AbstractJdbcIT.java | 11 +--
.../org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java | 98 ++++++++++++++++++++--
.../tarantool/jdbc/JdbcExceptionHandlingTest.java | 60 +++++++++++++
4 files changed, 215 insertions(+), 32 deletions(-)
create mode 100644 src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
index e43bcdc..c8879c9 100644
--- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
+++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
@@ -10,6 +10,7 @@ import java.sql.Types;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
import java.util.Map;
@@ -765,29 +766,53 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
@Override
public ResultSet getPrimaryKeys(String catalog, String schema, String table) throws SQLException {
- List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 2, Arrays.asList(table), 0, 1, 0);
+ final List<String> colNames = Arrays.asList(
+ "TABLE_CAT", "TABLE_SCHEM", "TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME");
- List<List<Object>> rows = new ArrayList<List<Object>>();
+ if (table == null || table.isEmpty())
+ return emptyResultSet(colNames);
+
+ try {
+ List spaces = connection.connection.select(_VSPACE, 2, Collections.singletonList(table), 0, 1, 0);
+
+ if (spaces == null || spaces.size() == 0)
+ return emptyResultSet(colNames);
- if (spaces != null && spaces.size() > 0) {
- List<Object> space = spaces.get(0);
- List<Map<String, Object>> fields = (List<Map<String, Object>>) space.get(FORMAT_IDX);
- List<List<Object>> indexes = (List<List<Object>>) connection.connection.select(_VINDEX, 0, Arrays.asList(space.get(SPACE_ID_IDX), 0), 0, 1, 0);
- List<Object> primaryKey = indexes.get(0);
- List<List<Object>> parts = (List<List<Object>>) primaryKey.get(INDEX_FORMAT_IDX);
+ List space = ensureType(List.class, spaces.get(0));
+ List fields = ensureType(List.class, space.get(FORMAT_IDX));
+ int spaceId = ensureType(Number.class, space.get(SPACE_ID_IDX)).intValue();
+ List indexes = connection.connection.select(_VINDEX, 0, Arrays.asList(spaceId, 0), 0, 1, 0);
+ List primaryKey = ensureType(List.class, indexes.get(0));
+ List parts = ensureType(List.class, primaryKey.get(INDEX_FORMAT_IDX));
+
+ List<List<Object>> rows = new ArrayList<List<Object>>();
for (int i = 0; i < parts.size(); i++) {
- List<Object> part = parts.get(i);
- int ordinal = ((Number) part.get(0)).intValue();
- String column = (String) fields.get(ordinal).get("name");
- rows.add(Arrays.asList(table, column, i + 1, "primary", primaryKey.get(NAME_IDX)));
+ // For native spaces, the 'parts' is 'List of Lists'.
+ // We only accept SQL spaces, for which the parts is 'List of Maps'.
+ Map part = checkType(Map.class, parts.get(i));
+ if (part == null)
+ return emptyResultSet(colNames);
+
+ int ordinal = ensureType(Number.class, part.get("field")).intValue();
+ Map field = ensureType(Map.class, fields.get(ordinal));
+ // The 'name' field is optional in the format structure. But it is present for SQL space.
+ String column = ensureType(String.class, field.get("name"));
+ rows.add(Arrays.asList(null, null, table, column, i + 1, primaryKey.get(NAME_IDX)));
}
+ // Sort results by column name.
+ Collections.sort(rows, new Comparator<List<Object>>() {
+ @Override
+ public int compare(List<Object> row0, List<Object> row1) {
+ String col0 = (String) row0.get(3);
+ String col1 = (String) row1.get(3);
+ return col0.compareTo(col1);
+ }
+ });
+ return new SQLNullResultSet((JDBCBridge.mock(colNames, rows)));
+ }
+ catch (Throwable t) {
+ throw new SQLException("Error processing metadata for table \"" + table + "\".", t);
}
- return new SQLNullResultSet((JDBCBridge.mock(
- Arrays.asList("TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME",
- //nulls
- "TABLE_CAT", "TABLE_SCHEM"
- ),
- rows)));
}
@Override
@@ -1026,4 +1051,21 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
public boolean isWrapperFor(Class<?> iface) throws SQLException {
throw new SQLFeatureNotSupportedException();
}
+
+ private static <T> T ensureType(Class<T> cls, Object v) throws Exception {
+ if (v == null || !cls.isAssignableFrom(v.getClass())) {
+ throw new Exception(String.format("Wrong value type '%s', expected '%s'.",
+ v == null ? "null" : v.getClass().getName(), cls.getName()));
+ }
+ return cls.cast(v);
+ }
+
+ private static <T> T checkType(Class<T> cls, Object v) {
+ return (v != null && cls.isAssignableFrom(v.getClass())) ? cls.cast(v) : null;
+ }
+
+ private ResultSet emptyResultSet(List<String> colNames) {
+ return new SQLNullResultSet((JDBCBridge.mock(colNames, Collections.<List<Object>>emptyList())));
+ }
+
}
diff --git a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
index 21e3f53..3df1aa4 100644
--- a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
+++ b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
@@ -32,9 +32,6 @@ public abstract class AbstractJdbcIT {
private static String URL = String.format("tarantool://%s:%d?user=%s&password=%s", host, port, user, pass);
private static String[] initSql = new String[] {
- "DROP TABLE IF EXISTS test",
- "DROP TABLE IF EXISTS test_types",
-
"CREATE TABLE test(id INT PRIMARY KEY, val VARCHAR(100))",
"INSERT INTO test VALUES (1, 'one'), (2, 'two'), (3, 'three')",
@@ -78,12 +75,15 @@ public abstract class AbstractJdbcIT {
"X'010203040506'," + //LONGVARBINARY
"'1983-03-14'," + //DATE
"'12:01:06'," + //TIME
- "129479994)" //TIMESTAMP
+ "129479994)", //TIMESTAMP
+
+ "CREATE TABLE test_compound(id1 INT, id2 INT, val VARCHAR(100), PRIMARY KEY (id2, id1))"
};
private static String[] cleanSql = new String[] {
"DROP TABLE IF EXISTS test",
- "DROP TABLE IF EXISTS test_types"
+ "DROP TABLE IF EXISTS test_types",
+ "DROP TABLE IF EXISTS test_compound"
};
static Object[] testRow = new Object[] {
@@ -112,6 +112,7 @@ public abstract class AbstractJdbcIT {
@BeforeAll
public static void setupEnv() throws Exception {
+ sqlExec(cleanSql);
sqlExec(initSql);
}
diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
index b934bbe..78ff89b 100644
--- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
+++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
@@ -1,11 +1,11 @@
package org.tarantool.jdbc;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.BeforeEach;
import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -45,6 +45,9 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
assertTrue(rs.next());
assertEquals("TEST_TYPES", rs.getString("TABLE_NAME"));
+ assertTrue(rs.next());
+ assertEquals("TEST_COMPOUND", rs.getString("TABLE_NAME"));
+
assertFalse(rs.next());
rs.close();
@@ -84,23 +87,100 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
rs.close();
}
- @Disabled(value="Test ignored, issue#41")
@Test
- public void testGetPrimaryKeys() throws SQLException {
+ public void testGetPrimaryKeys() throws Exception {
ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
assertNotNull(rs);
assertTrue(rs.next());
- assertNull(rs.getString("TABLE_CAT"));
- assertNull(rs.getString("TABLE_SCHEM"));
- assertEquals("TEST", rs.getString("TABLE_NAME"));
- assertEquals("ID", rs.getString("COLUMN_NAME"));
- assertEquals(1, rs.getInt("KEY_SEQ"));
- assertEquals("pk_unnamed_TEST_1", rs.getString("PK_NAME"));
+ checkGetPrimaryKeysRow(rs, "TEST", "ID", "pk_unnamed_TEST_1", 1);
+
+ assertFalse(rs.next());
+
+ rs.close();
+ }
+
+ @Test
+ public void testGetPrimaryKeysCompound() throws Exception {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "TEST_COMPOUND");
+
+ assertNotNull(rs);
+ assertTrue(rs.next());
+ checkGetPrimaryKeysRow(rs, "TEST_COMPOUND", "ID1", "pk_unnamed_TEST_COMPOUND_1", 2);
+
+ assertTrue(rs.next());
+ checkGetPrimaryKeysRow(rs, "TEST_COMPOUND", "ID2", "pk_unnamed_TEST_COMPOUND_1", 1);
+
+ assertFalse(rs.next());
+ rs.close();
+ }
+
+ @Test
+ public void testGetPrimaryKeysIgnoresCatalogSchema() throws Exception {
+ String[] vals = new String[] {null, "", "IGNORE"};
+ for (String cat : vals) {
+ for (String schema : vals) {
+ ResultSet rs = meta.getPrimaryKeys(cat, schema, "TEST");
+
+ assertNotNull(rs);
+ assertTrue(rs.next());
+ checkGetPrimaryKeysRow(rs, "TEST", "ID", "pk_unnamed_TEST_1", 1);
+ assertFalse(rs.next());
+ rs.close();
+ }
+ }
+ }
+
+ @Test
+ public void testGetPrimaryKeysNotFound() throws SQLException {
+ String[] tables = new String[] {null, "", "NOSUCHTABLE"};
+ for (String t : tables) {
+ ResultSet rs = meta.getPrimaryKeys(null, null, t);
+ assertNotNull(rs);
+ assertFalse(rs.next());
+ rs.close();
+ }
+ }
+
+ @Test
+ public void testGetPrimaryKeyNonSQLSpace() throws SQLException {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "_vspace");
+ assertNotNull(rs);
assertFalse(rs.next());
+ rs.close();
+ }
+ @Test
+ public void testGetPrimaryKeysOrderOfColumns() throws SQLException {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
+ assertNotNull(rs);
+ ResultSetMetaData rsMeta = rs.getMetaData();
+ assertEquals(6, rsMeta.getColumnCount());
+ assertEquals("TABLE_CAT", rsMeta.getColumnName(1));
+ assertEquals("TABLE_SCHEM", rsMeta.getColumnName(2));
+ assertEquals("TABLE_NAME", rsMeta.getColumnName(3));
+ assertEquals("COLUMN_NAME", rsMeta.getColumnName(4));
+ assertEquals("KEY_SEQ", rsMeta.getColumnName(5));
+ assertEquals("PK_NAME", rsMeta.getColumnName(6));
rs.close();
}
+
+ private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
+ throws Exception {
+ assertNull(rs.getString("TABLE_CAT"));
+ assertNull(rs.getString("TABLE_SCHEM"));
+ assertEquals(table, rs.getString("TABLE_NAME"));
+ assertEquals(colName, rs.getString("COLUMN_NAME"));
+ assertEquals(seq, rs.getInt("KEY_SEQ"));
+ assertEquals(pkName, rs.getString("PK_NAME"));
+
+ assertNull(rs.getString(1));
+ assertNull(rs.getString(2));
+ assertEquals(table, rs.getString(3));
+ assertEquals(colName, rs.getString(4));
+ assertEquals(seq, rs.getInt(5));
+ assertEquals(pkName, rs.getString(6));
+ }
}
diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
new file mode 100644
index 0000000..39d6326
--- /dev/null
+++ b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
@@ -0,0 +1,60 @@
+package org.tarantool.jdbc;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
+import org.tarantool.TarantoolConnection;
+
+import java.sql.DatabaseMetaData;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Properties;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.FORMAT_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.INDEX_FORMAT_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACE_ID_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata._VINDEX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata._VSPACE;
+
+public class JdbcExceptionHandlingTest {
+ /**
+ * Simulates meta parsing error: missing "name" field in a space format for the primary key.
+ *
+ * @throws Exception on failure.
+ */
+ @Test
+ public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws Exception {
+ TarantoolConnection tntCon = mock(TarantoolConnection.class);
+ SQLConnection conn = new SQLConnection(tntCon, "", new Properties());
+
+ Object[] spc = new Object[7];
+ spc[FORMAT_IDX] = Collections.singletonList(new HashMap<String, Object>());
+ spc[SPACE_ID_IDX] = 1000;
+
+ doReturn(Collections.singletonList(Arrays.asList(spc))).when(tntCon)
+ .select(_VSPACE, 2, Collections.singletonList("TEST"), 0, 1, 0);
+
+ Object[] idx = new Object[6];
+ idx[INDEX_FORMAT_IDX] = Collections.singletonList(
+ new HashMap<String, Object>() {{ put("field", 0);}});
+
+ doReturn(Collections.singletonList(Arrays.asList(idx))).when(tntCon)
+ .select(_VINDEX, 0, Arrays.asList(1000, 0), 0, 1, 0);
+
+ final DatabaseMetaData meta = conn.getMetaData();
+
+ Throwable t = assertThrows(SQLException.class, new Executable() {
+ @Override
+ public void execute() throws Throwable {
+ meta.getPrimaryKeys(null, null, "TEST");
+ }
+ }, "Error processing metadata for table \"TEST\".");
+
+ assertTrue(t.getCause().getMessage().contains("Wrong value type"));
+ }
+}
--
1.8.3.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-10-01 13:04 ` [tarantool-patches] " Alexander Turenko
2018-10-02 17:09 ` Sergei Kalashnikov
@ 2018-10-09 9:21 ` Sergei Kalashnikov
2018-10-09 15:40 ` Alexander Turenko
1 sibling, 1 reply; 11+ messages in thread
From: Sergei Kalashnikov @ 2018-10-09 9:21 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Resending this mail as something went wrong with it the first time.
Hi Alexander!
Thanks for the review. Please see my replies inline below.
On Mon, Oct 01, 2018 at 04:04:27PM +0300, Alexander Turenko wrote:
> Hi!
>
> Sorry for the late response.
>
> From [1]:
>
> > Retrieves a description of the given table's primary key columns. They are ordered by COLUMN_NAME.
>
> That is strange, but that is. Maybe we should also check the standard.
>
> [1]: https://docs.oracle.com/javase/7/docs/api/java/sql/DatabaseMetaData.html#getPrimaryKeys(java.lang.String,%20java.lang.String,%20java.lang.String)
The standard lacks such details. It relies on the underlying Java API docs.
And some vendors apply different ordering (e.g. table_name, pk_name, key_seq).
I added the sorting in revised patch (please note how the definition of
'test_compound' table has changed in tests).
I have noticed another deviation from specification. The order in which columns are
returned in the resultset is not correct. The TABLE_CAT and TABLE_SCHEM must go first.
I have fixed this as well. Plz see the new test 'testGetPrimaryKeysOrderOfColumns'.
>
> Tests are ok for me.
>
> Maybe we also should set some defined behaviour for the case when
> 'catalog' and 'schema' are given for getPrimaryKeys. I think we should
> ignore them and return nulls in TABLE_CAT and TABLE_SCHEM. I think we
> should have corresponding test case.
I agree. I see that, for instance, postgreSQL JDBC are doing that for catalog.
I added a test 'testGetPrimaryKeysIgnoresCatalogSchema' for that.
> >
> > diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
> > index e43bcdc..790bbe7 100644
> > --- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
> > +++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
> > @@ -774,12 +774,12 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
> > List<Map<String, Object>> fields = (List<Map<String, Object>>) space.get(FORMAT_IDX);
> > List<List<Object>> indexes = (List<List<Object>>) connection.connection.select(_VINDEX, 0, Arrays.asList(space.get(SPACE_ID_IDX), 0), 0, 1, 0);
> > List<Object> primaryKey = indexes.get(0);
> > - List<List<Object>> parts = (List<List<Object>>) primaryKey.get(INDEX_FORMAT_IDX);
> > + List<Map<String, Object>> parts = (List<Map<String, Object>>) primaryKey.get(INDEX_FORMAT_IDX);
>
> I think we should check the type to give a proper error message in the
> case when a user give 'native' tarantool space in parameters.
> Corresponding test case should be added.
>
> Don't sure how it will be wrapped in Java, I just about the following:
>
> 6th field of _vspace tuple for a 'native' space:
>
> [[0, 'unsigned'], [1, 'unsigned']]
>
> (Don't sure whether it is list or map in the binary protocol; likely
> map.)
>
> 6th field of _vspace tuple for a sql space:
>
> [{'sort_order': 'asc', 'type': 'scalar', 'field': 0,
> 'nullable_action': 'abort', 'is_nullable': false}]
>
> What would going on if a user give us 'native' space name?
I've made some tests. For the 6th field of '_vspace' tuple, I've got the same 'List<Map>'
in both native and sql space cases, the difference was only in map contents.
However for the 5th field of '_vindex' tuple, it is 'List<List>' for the native space
and 'List<Map>' for the sql space.
So, I added extensive type checking and throw an exception in case unexpected type is encountered.
See tests 'testGetPrimaryKeyNonSQLSpace' and 'testDatabaseMetaDataGetPrimaryKeysFormatError'.
Are you certain that we should return the error in case we have been given a native space?
I went for empty resultset instead. Hope you are OK with that.
>
> > for (int i = 0; i < parts.size(); i++) {
> > - List<Object> part = parts.get(i);
> > - int ordinal = ((Number) part.get(0)).intValue();
> > + Map<String, Object> part = parts.get(i);
> > + int ordinal = ((Number) part.get("field")).intValue();
> > String column = (String) fields.get(ordinal).get("name");
>
> Just note: it is not mandatory to space format fields to have a name,
> but SQL spaces have them. Maybe a comment needed here.
>
OK, done.
Please find below the fixed patch.
Commit 705c3ba3746cf5e196d9c58419348936817ea327
Fixed several mistakes in get primary keys metadata API.
Corrected type mismatch when parsing server response.
Added sorting of result rows by column name.
Fixed order of columns in result set.
Wrapped errors into SQLException.
Improved test coverage.
Closes #41
---
https://github.com/tarantool/tarantool-java/issues/41
https://github.com/ztarvos/tarantool-java/commits/gh-41-fix-jdbc-pk-meta
.../org/tarantool/jdbc/SQLDatabaseMetadata.java | 78 +++++++++++++----
.../java/org/tarantool/jdbc/AbstractJdbcIT.java | 11 +--
.../org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java | 98 ++++++++++++++++++++--
.../tarantool/jdbc/JdbcExceptionHandlingTest.java | 60 +++++++++++++
4 files changed, 215 insertions(+), 32 deletions(-)
create mode 100644 src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
index e43bcdc..c8879c9 100644
--- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
+++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
@@ -10,6 +10,7 @@ import java.sql.Types;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
import java.util.Map;
@@ -765,29 +766,53 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
@Override
public ResultSet getPrimaryKeys(String catalog, String schema, String table) throws SQLException {
- List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 2, Arrays.asList(table), 0, 1, 0);
+ final List<String> colNames = Arrays.asList(
+ "TABLE_CAT", "TABLE_SCHEM", "TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME");
- List<List<Object>> rows = new ArrayList<List<Object>>();
+ if (table == null || table.isEmpty())
+ return emptyResultSet(colNames);
+
+ try {
+ List spaces = connection.connection.select(_VSPACE, 2, Collections.singletonList(table), 0, 1, 0);
+
+ if (spaces == null || spaces.size() == 0)
+ return emptyResultSet(colNames);
- if (spaces != null && spaces.size() > 0) {
- List<Object> space = spaces.get(0);
- List<Map<String, Object>> fields = (List<Map<String, Object>>) space.get(FORMAT_IDX);
- List<List<Object>> indexes = (List<List<Object>>) connection.connection.select(_VINDEX, 0, Arrays.asList(space.get(SPACE_ID_IDX), 0), 0, 1, 0);
- List<Object> primaryKey = indexes.get(0);
- List<List<Object>> parts = (List<List<Object>>) primaryKey.get(INDEX_FORMAT_IDX);
+ List space = ensureType(List.class, spaces.get(0));
+ List fields = ensureType(List.class, space.get(FORMAT_IDX));
+ int spaceId = ensureType(Number.class, space.get(SPACE_ID_IDX)).intValue();
+ List indexes = connection.connection.select(_VINDEX, 0, Arrays.asList(spaceId, 0), 0, 1, 0);
+ List primaryKey = ensureType(List.class, indexes.get(0));
+ List parts = ensureType(List.class, primaryKey.get(INDEX_FORMAT_IDX));
+
+ List<List<Object>> rows = new ArrayList<List<Object>>();
for (int i = 0; i < parts.size(); i++) {
- List<Object> part = parts.get(i);
- int ordinal = ((Number) part.get(0)).intValue();
- String column = (String) fields.get(ordinal).get("name");
- rows.add(Arrays.asList(table, column, i + 1, "primary", primaryKey.get(NAME_IDX)));
+ // For native spaces, the 'parts' is 'List of Lists'.
+ // We only accept SQL spaces, for which the parts is 'List of Maps'.
+ Map part = checkType(Map.class, parts.get(i));
+ if (part == null)
+ return emptyResultSet(colNames);
+
+ int ordinal = ensureType(Number.class, part.get("field")).intValue();
+ Map field = ensureType(Map.class, fields.get(ordinal));
+ // The 'name' field is optional in the format structure. But it is present for SQL space.
+ String column = ensureType(String.class, field.get("name"));
+ rows.add(Arrays.asList(null, null, table, column, i + 1, primaryKey.get(NAME_IDX)));
}
+ // Sort results by column name.
+ Collections.sort(rows, new Comparator<List<Object>>() {
+ @Override
+ public int compare(List<Object> row0, List<Object> row1) {
+ String col0 = (String) row0.get(3);
+ String col1 = (String) row1.get(3);
+ return col0.compareTo(col1);
+ }
+ });
+ return new SQLNullResultSet((JDBCBridge.mock(colNames, rows)));
+ }
+ catch (Throwable t) {
+ throw new SQLException("Error processing metadata for table \"" + table + "\".", t);
}
- return new SQLNullResultSet((JDBCBridge.mock(
- Arrays.asList("TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME",
- //nulls
- "TABLE_CAT", "TABLE_SCHEM"
- ),
- rows)));
}
@Override
@@ -1026,4 +1051,21 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
public boolean isWrapperFor(Class<?> iface) throws SQLException {
throw new SQLFeatureNotSupportedException();
}
+
+ private static <T> T ensureType(Class<T> cls, Object v) throws Exception {
+ if (v == null || !cls.isAssignableFrom(v.getClass())) {
+ throw new Exception(String.format("Wrong value type '%s', expected '%s'.",
+ v == null ? "null" : v.getClass().getName(), cls.getName()));
+ }
+ return cls.cast(v);
+ }
+
+ private static <T> T checkType(Class<T> cls, Object v) {
+ return (v != null && cls.isAssignableFrom(v.getClass())) ? cls.cast(v) : null;
+ }
+
+ private ResultSet emptyResultSet(List<String> colNames) {
+ return new SQLNullResultSet((JDBCBridge.mock(colNames, Collections.<List<Object>>emptyList())));
+ }
+
}
diff --git a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
index 21e3f53..3df1aa4 100644
--- a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
+++ b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
@@ -32,9 +32,6 @@ public abstract class AbstractJdbcIT {
private static String URL = String.format("tarantool://%s:%d?user=%s&password=%s", host, port, user, pass);
private static String[] initSql = new String[] {
- "DROP TABLE IF EXISTS test",
- "DROP TABLE IF EXISTS test_types",
-
"CREATE TABLE test(id INT PRIMARY KEY, val VARCHAR(100))",
"INSERT INTO test VALUES (1, 'one'), (2, 'two'), (3, 'three')",
@@ -78,12 +75,15 @@ public abstract class AbstractJdbcIT {
"X'010203040506'," + //LONGVARBINARY
"'1983-03-14'," + //DATE
"'12:01:06'," + //TIME
- "129479994)" //TIMESTAMP
+ "129479994)", //TIMESTAMP
+
+ "CREATE TABLE test_compound(id1 INT, id2 INT, val VARCHAR(100), PRIMARY KEY (id2, id1))"
};
private static String[] cleanSql = new String[] {
"DROP TABLE IF EXISTS test",
- "DROP TABLE IF EXISTS test_types"
+ "DROP TABLE IF EXISTS test_types",
+ "DROP TABLE IF EXISTS test_compound"
};
static Object[] testRow = new Object[] {
@@ -112,6 +112,7 @@ public abstract class AbstractJdbcIT {
@BeforeAll
public static void setupEnv() throws Exception {
+ sqlExec(cleanSql);
sqlExec(initSql);
}
diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
index b934bbe..78ff89b 100644
--- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
+++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
@@ -1,11 +1,11 @@
package org.tarantool.jdbc;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.BeforeEach;
import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -45,6 +45,9 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
assertTrue(rs.next());
assertEquals("TEST_TYPES", rs.getString("TABLE_NAME"));
+ assertTrue(rs.next());
+ assertEquals("TEST_COMPOUND", rs.getString("TABLE_NAME"));
+
assertFalse(rs.next());
rs.close();
@@ -84,23 +87,100 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
rs.close();
}
- @Disabled(value="Test ignored, issue#41")
@Test
- public void testGetPrimaryKeys() throws SQLException {
+ public void testGetPrimaryKeys() throws Exception {
ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
assertNotNull(rs);
assertTrue(rs.next());
- assertNull(rs.getString("TABLE_CAT"));
- assertNull(rs.getString("TABLE_SCHEM"));
- assertEquals("TEST", rs.getString("TABLE_NAME"));
- assertEquals("ID", rs.getString("COLUMN_NAME"));
- assertEquals(1, rs.getInt("KEY_SEQ"));
- assertEquals("pk_unnamed_TEST_1", rs.getString("PK_NAME"));
+ checkGetPrimaryKeysRow(rs, "TEST", "ID", "pk_unnamed_TEST_1", 1);
+
+ assertFalse(rs.next());
+
+ rs.close();
+ }
+
+ @Test
+ public void testGetPrimaryKeysCompound() throws Exception {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "TEST_COMPOUND");
+
+ assertNotNull(rs);
+ assertTrue(rs.next());
+ checkGetPrimaryKeysRow(rs, "TEST_COMPOUND", "ID1", "pk_unnamed_TEST_COMPOUND_1", 2);
+
+ assertTrue(rs.next());
+ checkGetPrimaryKeysRow(rs, "TEST_COMPOUND", "ID2", "pk_unnamed_TEST_COMPOUND_1", 1);
+
+ assertFalse(rs.next());
+ rs.close();
+ }
+
+ @Test
+ public void testGetPrimaryKeysIgnoresCatalogSchema() throws Exception {
+ String[] vals = new String[] {null, "", "IGNORE"};
+ for (String cat : vals) {
+ for (String schema : vals) {
+ ResultSet rs = meta.getPrimaryKeys(cat, schema, "TEST");
+
+ assertNotNull(rs);
+ assertTrue(rs.next());
+ checkGetPrimaryKeysRow(rs, "TEST", "ID", "pk_unnamed_TEST_1", 1);
+ assertFalse(rs.next());
+ rs.close();
+ }
+ }
+ }
+
+ @Test
+ public void testGetPrimaryKeysNotFound() throws SQLException {
+ String[] tables = new String[] {null, "", "NOSUCHTABLE"};
+ for (String t : tables) {
+ ResultSet rs = meta.getPrimaryKeys(null, null, t);
+ assertNotNull(rs);
+ assertFalse(rs.next());
+ rs.close();
+ }
+ }
+
+ @Test
+ public void testGetPrimaryKeyNonSQLSpace() throws SQLException {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "_vspace");
+ assertNotNull(rs);
assertFalse(rs.next());
+ rs.close();
+ }
+ @Test
+ public void testGetPrimaryKeysOrderOfColumns() throws SQLException {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
+ assertNotNull(rs);
+ ResultSetMetaData rsMeta = rs.getMetaData();
+ assertEquals(6, rsMeta.getColumnCount());
+ assertEquals("TABLE_CAT", rsMeta.getColumnName(1));
+ assertEquals("TABLE_SCHEM", rsMeta.getColumnName(2));
+ assertEquals("TABLE_NAME", rsMeta.getColumnName(3));
+ assertEquals("COLUMN_NAME", rsMeta.getColumnName(4));
+ assertEquals("KEY_SEQ", rsMeta.getColumnName(5));
+ assertEquals("PK_NAME", rsMeta.getColumnName(6));
rs.close();
}
+
+ private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
+ throws Exception {
+ assertNull(rs.getString("TABLE_CAT"));
+ assertNull(rs.getString("TABLE_SCHEM"));
+ assertEquals(table, rs.getString("TABLE_NAME"));
+ assertEquals(colName, rs.getString("COLUMN_NAME"));
+ assertEquals(seq, rs.getInt("KEY_SEQ"));
+ assertEquals(pkName, rs.getString("PK_NAME"));
+
+ assertNull(rs.getString(1));
+ assertNull(rs.getString(2));
+ assertEquals(table, rs.getString(3));
+ assertEquals(colName, rs.getString(4));
+ assertEquals(seq, rs.getInt(5));
+ assertEquals(pkName, rs.getString(6));
+ }
}
diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
new file mode 100644
index 0000000..39d6326
--- /dev/null
+++ b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
@@ -0,0 +1,60 @@
+package org.tarantool.jdbc;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
+import org.tarantool.TarantoolConnection;
+
+import java.sql.DatabaseMetaData;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Properties;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.FORMAT_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.INDEX_FORMAT_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACE_ID_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata._VINDEX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata._VSPACE;
+
+public class JdbcExceptionHandlingTest {
+ /**
+ * Simulates meta parsing error: missing "name" field in a space format for the primary key.
+ *
+ * @throws Exception on failure.
+ */
+ @Test
+ public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws Exception {
+ TarantoolConnection tntCon = mock(TarantoolConnection.class);
+ SQLConnection conn = new SQLConnection(tntCon, "", new Properties());
+
+ Object[] spc = new Object[7];
+ spc[FORMAT_IDX] = Collections.singletonList(new HashMap<String, Object>());
+ spc[SPACE_ID_IDX] = 1000;
+
+ doReturn(Collections.singletonList(Arrays.asList(spc))).when(tntCon)
+ .select(_VSPACE, 2, Collections.singletonList("TEST"), 0, 1, 0);
+
+ Object[] idx = new Object[6];
+ idx[INDEX_FORMAT_IDX] = Collections.singletonList(
+ new HashMap<String, Object>() {{ put("field", 0);}});
+
+ doReturn(Collections.singletonList(Arrays.asList(idx))).when(tntCon)
+ .select(_VINDEX, 0, Arrays.asList(1000, 0), 0, 1, 0);
+
+ final DatabaseMetaData meta = conn.getMetaData();
+
+ Throwable t = assertThrows(SQLException.class, new Executable() {
+ @Override
+ public void execute() throws Throwable {
+ meta.getPrimaryKeys(null, null, "TEST");
+ }
+ }, "Error processing metadata for table \"TEST\".");
+
+ assertTrue(t.getCause().getMessage().contains("Wrong value type"));
+ }
+}
--
1.8.3.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-10-09 9:21 ` Sergei Kalashnikov
@ 2018-10-09 15:40 ` Alexander Turenko
2018-10-10 8:14 ` Sergei Kalashnikov
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Turenko @ 2018-10-09 15:40 UTC (permalink / raw)
To: Sergei Kalashnikov; +Cc: tarantool-patches
Hi, Sergei!
The patch is okay for me. I have a couple of minor questions below, can you
please clarify them to me before we'll going to merge this patch?
WBR, Alexander Turenko.
> I have noticed another deviation from specification. The order in which columns are
> returned in the resultset is not correct. The TABLE_CAT and TABLE_SCHEM must go first.
> I have fixed this as well. Plz see the new test 'testGetPrimaryKeysOrderOfColumns'.
Good catch, thanks!
> > I think we should check the type to give a proper error message in the
> > case when a user give 'native' tarantool space in parameters.
> > Corresponding test case should be added.
> >
> > Don't sure how it will be wrapped in Java, I just about the following:
> >
> > 6th field of _vspace tuple for a 'native' space:
> >
> > [[0, 'unsigned'], [1, 'unsigned']]
> >
> > (Don't sure whether it is list or map in the binary protocol; likely
> > map.)
> >
> > 6th field of _vspace tuple for a sql space:
> >
> > [{'sort_order': 'asc', 'type': 'scalar', 'field': 0,
> > 'nullable_action': 'abort', 'is_nullable': false}]
> >
> > What would going on if a user give us 'native' space name?
>
> I've made some tests. For the 6th field of '_vspace' tuple, I've got the same 'List<Map>'
> in both native and sql space cases, the difference was only in map contents.
> However for the 5th field of '_vindex' tuple, it is 'List<List>' for the native space
> and 'List<Map>' for the sql space.
>
> So, I added extensive type checking and throw an exception in case unexpected type is encountered.
> See tests 'testGetPrimaryKeyNonSQLSpace' and 'testDatabaseMetaDataGetPrimaryKeysFormatError'.
>
> Are you certain that we should return the error in case we have been given a native space?
> I went for empty resultset instead. Hope you are OK with that.
A 'native' space is not visible in Tarantool SQL as a table, so we
likely should behave in the similar way as in no such table situation.
Don't found any information whether we should raise an error in case of
non-exists table or return empty result set from the API reference. What
other vendors do? Postgresql seems ([1]) to return empty result set.
[1]: https://github.com/pgjdbc/pgjdbc/blob/08631ccdabdb8ba6d52f398e2b0b46a9cf0cafbf/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java#L2060
So it is ok if it is really common way to return metainformation (across API
methods as well as across vendors). Are you have any extra info / experience on
that?
> + private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
> + throws Exception {
Why `throws Exception`, but not `SQLException`? The following patch
runs smoothly for me:
```
diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
index 78ff89b..8577b23 100644
--- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
+++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
@@ -88,7 +88,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
}
@Test
- public void testGetPrimaryKeys() throws Exception {
+ public void testGetPrimaryKeys() throws SQLException {
ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
assertNotNull(rs);
@@ -102,7 +102,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
}
@Test
- public void testGetPrimaryKeysCompound() throws Exception {
+ public void testGetPrimaryKeysCompound() throws SQLException {
ResultSet rs = meta.getPrimaryKeys(null, null, "TEST_COMPOUND");
assertNotNull(rs);
@@ -118,7 +118,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
}
@Test
- public void testGetPrimaryKeysIgnoresCatalogSchema() throws Exception {
+ public void testGetPrimaryKeysIgnoresCatalogSchema() throws SQLException {
String[] vals = new String[] {null, "", "IGNORE"};
for (String cat : vals) {
for (String schema : vals) {
@@ -168,7 +168,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
}
private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
- throws Exception {
+ throws SQLException {
assertNull(rs.getString("TABLE_CAT"));
assertNull(rs.getString("TABLE_SCHEM"));
assertEquals(table, rs.getString("TABLE_NAME"));
```
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-10-09 15:40 ` Alexander Turenko
@ 2018-10-10 8:14 ` Sergei Kalashnikov
2018-10-10 10:19 ` Alexander Turenko
0 siblings, 1 reply; 11+ messages in thread
From: Sergei Kalashnikov @ 2018-10-10 8:14 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Alexander, please find my answers inline below.
On Tue, Oct 09, 2018 at 06:40:06PM +0300, Alexander Turenko wrote:
> Hi, Sergei!
>
> The patch is okay for me. I have a couple of minor questions below, can you
> please clarify them to me before we'll going to merge this patch?
>
> WBR, Alexander Turenko.
>
> > I have noticed another deviation from specification. The order in which columns are
> > returned in the resultset is not correct. The TABLE_CAT and TABLE_SCHEM must go first.
> > I have fixed this as well. Plz see the new test 'testGetPrimaryKeysOrderOfColumns'.
>
> Good catch, thanks!
>
> > > I think we should check the type to give a proper error message in the
> > > case when a user give 'native' tarantool space in parameters.
> > > Corresponding test case should be added.
> > >
> > > Don't sure how it will be wrapped in Java, I just about the following:
> > >
> > > 6th field of _vspace tuple for a 'native' space:
> > >
> > > [[0, 'unsigned'], [1, 'unsigned']]
> > >
> > > (Don't sure whether it is list or map in the binary protocol; likely
> > > map.)
> > >
> > > 6th field of _vspace tuple for a sql space:
> > >
> > > [{'sort_order': 'asc', 'type': 'scalar', 'field': 0,
> > > 'nullable_action': 'abort', 'is_nullable': false}]
> > >
> > > What would going on if a user give us 'native' space name?
> >
> > I've made some tests. For the 6th field of '_vspace' tuple, I've got the same 'List<Map>'
> > in both native and sql space cases, the difference was only in map contents.
> > However for the 5th field of '_vindex' tuple, it is 'List<List>' for the native space
> > and 'List<Map>' for the sql space.
> >
> > So, I added extensive type checking and throw an exception in case unexpected type is encountered.
> > See tests 'testGetPrimaryKeyNonSQLSpace' and 'testDatabaseMetaDataGetPrimaryKeysFormatError'.
> >
> > Are you certain that we should return the error in case we have been given a native space?
> > I went for empty resultset instead. Hope you are OK with that.
>
> A 'native' space is not visible in Tarantool SQL as a table, so we
> likely should behave in the similar way as in no such table situation.
>
> Don't found any information whether we should raise an error in case of
> non-exists table or return empty result set from the API reference. What
> other vendors do? Postgresql seems ([1]) to return empty result set.
>
> [1]: https://github.com/pgjdbc/pgjdbc/blob/08631ccdabdb8ba6d52f398e2b0b46a9cf0cafbf/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java#L2060
>
> So it is ok if it is really common way to return metainformation (across API
> methods as well as across vendors). Are you have any extra info / experience on
> that?
>
I just did a quick test with live Oracle instance and it returns empty
resultset for unknown table. So I guess, it is correct.
> > + private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
> > + throws Exception {
>
> Why `throws Exception`, but not `SQLException`? The following patch
> runs smoothly for me:
>
> ```
> diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
> index 78ff89b..8577b23 100644
> --- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
> +++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
> @@ -88,7 +88,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> }
>
> @Test
> - public void testGetPrimaryKeys() throws Exception {
> + public void testGetPrimaryKeys() throws SQLException {
> ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
>
> assertNotNull(rs);
> @@ -102,7 +102,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> }
>
> @Test
> - public void testGetPrimaryKeysCompound() throws Exception {
> + public void testGetPrimaryKeysCompound() throws SQLException {
> ResultSet rs = meta.getPrimaryKeys(null, null, "TEST_COMPOUND");
>
> assertNotNull(rs);
> @@ -118,7 +118,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> }
>
> @Test
> - public void testGetPrimaryKeysIgnoresCatalogSchema() throws Exception {
> + public void testGetPrimaryKeysIgnoresCatalogSchema() throws SQLException {
> String[] vals = new String[] {null, "", "IGNORE"};
> for (String cat : vals) {
> for (String schema : vals) {
> @@ -168,7 +168,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> }
>
> private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
> - throws Exception {
> + throws SQLException {
> assertNull(rs.getString("TABLE_CAT"));
> assertNull(rs.getString("TABLE_SCHEM"));
> assertEquals(table, rs.getString("TABLE_NAME"));
> ```
Yes, it should be SQLException. Probably I was doing some experiments here and then forgot to clean it up.
Let me know if you want me to correct this with another revision of the patch.
--
Best Regards,
Sergei K.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-10-10 8:14 ` Sergei Kalashnikov
@ 2018-10-10 10:19 ` Alexander Turenko
2018-10-10 11:09 ` Sergei Kalashnikov
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Turenko @ 2018-10-10 10:19 UTC (permalink / raw)
To: Sergei Kalashnikov; +Cc: tarantool-patches
Hi!
The patch LGTM. I have the last minor concern re exception handling in a
test (see below) and then the patch can be pushed. Please, fix it and
ping me to merge the patch.
WBR, Alexander Turenko.
> > > > I think we should check the type to give a proper error message in the
> > > > case when a user give 'native' tarantool space in parameters.
> > > > Corresponding test case should be added.
> > > >
> > > > Don't sure how it will be wrapped in Java, I just about the following:
> > > >
> > > > 6th field of _vspace tuple for a 'native' space:
> > > >
> > > > [[0, 'unsigned'], [1, 'unsigned']]
> > > >
> > > > (Don't sure whether it is list or map in the binary protocol; likely
> > > > map.)
> > > >
> > > > 6th field of _vspace tuple for a sql space:
> > > >
> > > > [{'sort_order': 'asc', 'type': 'scalar', 'field': 0,
> > > > 'nullable_action': 'abort', 'is_nullable': false}]
> > > >
> > > > What would going on if a user give us 'native' space name?
> > >
> > > I've made some tests. For the 6th field of '_vspace' tuple, I've got the same 'List<Map>'
> > > in both native and sql space cases, the difference was only in map contents.
> > > However for the 5th field of '_vindex' tuple, it is 'List<List>' for the native space
> > > and 'List<Map>' for the sql space.
> > >
> > > So, I added extensive type checking and throw an exception in case unexpected type is encountered.
> > > See tests 'testGetPrimaryKeyNonSQLSpace' and 'testDatabaseMetaDataGetPrimaryKeysFormatError'.
> > >
> > > Are you certain that we should return the error in case we have been given a native space?
> > > I went for empty resultset instead. Hope you are OK with that.
> >
> > A 'native' space is not visible in Tarantool SQL as a table, so we
> > likely should behave in the similar way as in no such table situation.
> >
> > Don't found any information whether we should raise an error in case of
> > non-exists table or return empty result set from the API reference. What
> > other vendors do? Postgresql seems ([1]) to return empty result set.
> >
> > [1]: https://github.com/pgjdbc/pgjdbc/blob/08631ccdabdb8ba6d52f398e2b0b46a9cf0cafbf/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java#L2060
> >
> > So it is ok if it is really common way to return metainformation (across API
> > methods as well as across vendors). Are you have any extra info / experience on
> > that?
> >
>
> I just did a quick test with live Oracle instance and it returns empty
> resultset for unknown table. So I guess, it is correct.
>
So okay for me. Thanks for the clarification!
> > > + private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
> > > + throws Exception {
> >
> > Why `throws Exception`, but not `SQLException`? The following patch
> > runs smoothly for me:
> >
> > ```
> > diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
> > index 78ff89b..8577b23 100644
> > --- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
> > +++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
> > @@ -88,7 +88,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> > }
> >
> > @Test
> > - public void testGetPrimaryKeys() throws Exception {
> > + public void testGetPrimaryKeys() throws SQLException {
> > ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
> >
> > assertNotNull(rs);
> > @@ -102,7 +102,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> > }
> >
> > @Test
> > - public void testGetPrimaryKeysCompound() throws Exception {
> > + public void testGetPrimaryKeysCompound() throws SQLException {
> > ResultSet rs = meta.getPrimaryKeys(null, null, "TEST_COMPOUND");
> >
> > assertNotNull(rs);
> > @@ -118,7 +118,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> > }
> >
> > @Test
> > - public void testGetPrimaryKeysIgnoresCatalogSchema() throws Exception {
> > + public void testGetPrimaryKeysIgnoresCatalogSchema() throws SQLException {
> > String[] vals = new String[] {null, "", "IGNORE"};
> > for (String cat : vals) {
> > for (String schema : vals) {
> > @@ -168,7 +168,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> > }
> >
> > private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
> > - throws Exception {
> > + throws SQLException {
> > assertNull(rs.getString("TABLE_CAT"));
> > assertNull(rs.getString("TABLE_SCHEM"));
> > assertEquals(table, rs.getString("TABLE_NAME"));
> > ```
>
> Yes, it should be SQLException. Probably I was doing some experiments here and then forgot to clean it up.
> Let me know if you want me to correct this with another revision of the patch.
I think it is better to correct this thing. This is the last concern I
have.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-10-10 10:19 ` Alexander Turenko
@ 2018-10-10 11:09 ` Sergei Kalashnikov
2018-10-10 12:50 ` Alexander Turenko
0 siblings, 1 reply; 11+ messages in thread
From: Sergei Kalashnikov @ 2018-10-10 11:09 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Sure. Please find the amended patch at the very end of this mail.
On Wed, Oct 10, 2018 at 01:19:38PM +0300, Alexander Turenko wrote:
> Hi!
>
> The patch LGTM. I have the last minor concern re exception handling in a
> test (see below) and then the patch can be pushed. Please, fix it and
> ping me to merge the patch.
>
> WBR, Alexander Turenko.
>
> > > > > I think we should check the type to give a proper error message in the
> > > > > case when a user give 'native' tarantool space in parameters.
> > > > > Corresponding test case should be added.
> > > > >
> > > > > Don't sure how it will be wrapped in Java, I just about the following:
> > > > >
> > > > > 6th field of _vspace tuple for a 'native' space:
> > > > >
> > > > > [[0, 'unsigned'], [1, 'unsigned']]
> > > > >
> > > > > (Don't sure whether it is list or map in the binary protocol; likely
> > > > > map.)
> > > > >
> > > > > 6th field of _vspace tuple for a sql space:
> > > > >
> > > > > [{'sort_order': 'asc', 'type': 'scalar', 'field': 0,
> > > > > 'nullable_action': 'abort', 'is_nullable': false}]
> > > > >
> > > > > What would going on if a user give us 'native' space name?
> > > >
> > > > I've made some tests. For the 6th field of '_vspace' tuple, I've got the same 'List<Map>'
> > > > in both native and sql space cases, the difference was only in map contents.
> > > > However for the 5th field of '_vindex' tuple, it is 'List<List>' for the native space
> > > > and 'List<Map>' for the sql space.
> > > >
> > > > So, I added extensive type checking and throw an exception in case unexpected type is encountered.
> > > > See tests 'testGetPrimaryKeyNonSQLSpace' and 'testDatabaseMetaDataGetPrimaryKeysFormatError'.
> > > >
> > > > Are you certain that we should return the error in case we have been given a native space?
> > > > I went for empty resultset instead. Hope you are OK with that.
> > >
> > > A 'native' space is not visible in Tarantool SQL as a table, so we
> > > likely should behave in the similar way as in no such table situation.
> > >
> > > Don't found any information whether we should raise an error in case of
> > > non-exists table or return empty result set from the API reference. What
> > > other vendors do? Postgresql seems ([1]) to return empty result set.
> > >
> > > [1]: https://github.com/pgjdbc/pgjdbc/blob/08631ccdabdb8ba6d52f398e2b0b46a9cf0cafbf/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java#L2060
> > >
> > > So it is ok if it is really common way to return metainformation (across API
> > > methods as well as across vendors). Are you have any extra info / experience on
> > > that?
> > >
> >
> > I just did a quick test with live Oracle instance and it returns empty
> > resultset for unknown table. So I guess, it is correct.
> >
>
> So okay for me. Thanks for the clarification!
>
> > > > + private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
> > > > + throws Exception {
> > >
> > > Why `throws Exception`, but not `SQLException`? The following patch
> > > runs smoothly for me:
> > >
> > > ```
> > > diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
> > > index 78ff89b..8577b23 100644
> > > --- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
> > > +++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
> > > @@ -88,7 +88,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> > > }
> > >
> > > @Test
> > > - public void testGetPrimaryKeys() throws Exception {
> > > + public void testGetPrimaryKeys() throws SQLException {
> > > ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
> > >
> > > assertNotNull(rs);
> > > @@ -102,7 +102,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> > > }
> > >
> > > @Test
> > > - public void testGetPrimaryKeysCompound() throws Exception {
> > > + public void testGetPrimaryKeysCompound() throws SQLException {
> > > ResultSet rs = meta.getPrimaryKeys(null, null, "TEST_COMPOUND");
> > >
> > > assertNotNull(rs);
> > > @@ -118,7 +118,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> > > }
> > >
> > > @Test
> > > - public void testGetPrimaryKeysIgnoresCatalogSchema() throws Exception {
> > > + public void testGetPrimaryKeysIgnoresCatalogSchema() throws SQLException {
> > > String[] vals = new String[] {null, "", "IGNORE"};
> > > for (String cat : vals) {
> > > for (String schema : vals) {
> > > @@ -168,7 +168,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
> > > }
> > >
> > > private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
> > > - throws Exception {
> > > + throws SQLException {
> > > assertNull(rs.getString("TABLE_CAT"));
> > > assertNull(rs.getString("TABLE_SCHEM"));
> > > assertEquals(table, rs.getString("TABLE_NAME"));
> > > ```
> >
> > Yes, it should be SQLException. Probably I was doing some experiments here and then forgot to clean it up.
> > Let me know if you want me to correct this with another revision of the patch.
>
> I think it is better to correct this thing. This is the last concern I
> have.
Below is the amended patch.
Commit 1678627898a891fb4cae4bd4012b489cfbbd12b5
jdbc: fix primary keys meta retrieval
Fixed several mistakes in get primary keys metadata API.
Corrected type mismatch when parsing server response.
Added sorting of result rows by column name.
Fixed order of columns in result set.
Wrapped errors into SQLException.
Improved test coverage.
Closes #41
---
.../org/tarantool/jdbc/SQLDatabaseMetadata.java | 78 +++++++++++++----
.../java/org/tarantool/jdbc/AbstractJdbcIT.java | 11 +--
.../org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java | 98 ++++++++++++++++++++--
.../tarantool/jdbc/JdbcExceptionHandlingTest.java | 60 +++++++++++++
4 files changed, 215 insertions(+), 32 deletions(-)
create mode 100644 src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
index e43bcdc..c8879c9 100644
--- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
+++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java
@@ -10,6 +10,7 @@ import java.sql.Types;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
import java.util.Map;
@@ -765,29 +766,53 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
@Override
public ResultSet getPrimaryKeys(String catalog, String schema, String table) throws SQLException {
- List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 2, Arrays.asList(table), 0, 1, 0);
+ final List<String> colNames = Arrays.asList(
+ "TABLE_CAT", "TABLE_SCHEM", "TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME");
- List<List<Object>> rows = new ArrayList<List<Object>>();
+ if (table == null || table.isEmpty())
+ return emptyResultSet(colNames);
+
+ try {
+ List spaces = connection.connection.select(_VSPACE, 2, Collections.singletonList(table), 0, 1, 0);
+
+ if (spaces == null || spaces.size() == 0)
+ return emptyResultSet(colNames);
- if (spaces != null && spaces.size() > 0) {
- List<Object> space = spaces.get(0);
- List<Map<String, Object>> fields = (List<Map<String, Object>>) space.get(FORMAT_IDX);
- List<List<Object>> indexes = (List<List<Object>>) connection.connection.select(_VINDEX, 0, Arrays.asList(space.get(SPACE_ID_IDX), 0), 0, 1, 0);
- List<Object> primaryKey = indexes.get(0);
- List<List<Object>> parts = (List<List<Object>>) primaryKey.get(INDEX_FORMAT_IDX);
+ List space = ensureType(List.class, spaces.get(0));
+ List fields = ensureType(List.class, space.get(FORMAT_IDX));
+ int spaceId = ensureType(Number.class, space.get(SPACE_ID_IDX)).intValue();
+ List indexes = connection.connection.select(_VINDEX, 0, Arrays.asList(spaceId, 0), 0, 1, 0);
+ List primaryKey = ensureType(List.class, indexes.get(0));
+ List parts = ensureType(List.class, primaryKey.get(INDEX_FORMAT_IDX));
+
+ List<List<Object>> rows = new ArrayList<List<Object>>();
for (int i = 0; i < parts.size(); i++) {
- List<Object> part = parts.get(i);
- int ordinal = ((Number) part.get(0)).intValue();
- String column = (String) fields.get(ordinal).get("name");
- rows.add(Arrays.asList(table, column, i + 1, "primary", primaryKey.get(NAME_IDX)));
+ // For native spaces, the 'parts' is 'List of Lists'.
+ // We only accept SQL spaces, for which the parts is 'List of Maps'.
+ Map part = checkType(Map.class, parts.get(i));
+ if (part == null)
+ return emptyResultSet(colNames);
+
+ int ordinal = ensureType(Number.class, part.get("field")).intValue();
+ Map field = ensureType(Map.class, fields.get(ordinal));
+ // The 'name' field is optional in the format structure. But it is present for SQL space.
+ String column = ensureType(String.class, field.get("name"));
+ rows.add(Arrays.asList(null, null, table, column, i + 1, primaryKey.get(NAME_IDX)));
}
+ // Sort results by column name.
+ Collections.sort(rows, new Comparator<List<Object>>() {
+ @Override
+ public int compare(List<Object> row0, List<Object> row1) {
+ String col0 = (String) row0.get(3);
+ String col1 = (String) row1.get(3);
+ return col0.compareTo(col1);
+ }
+ });
+ return new SQLNullResultSet((JDBCBridge.mock(colNames, rows)));
+ }
+ catch (Throwable t) {
+ throw new SQLException("Error processing metadata for table \"" + table + "\".", t);
}
- return new SQLNullResultSet((JDBCBridge.mock(
- Arrays.asList("TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME",
- //nulls
- "TABLE_CAT", "TABLE_SCHEM"
- ),
- rows)));
}
@Override
@@ -1026,4 +1051,21 @@ public class SQLDatabaseMetadata implements DatabaseMetaData {
public boolean isWrapperFor(Class<?> iface) throws SQLException {
throw new SQLFeatureNotSupportedException();
}
+
+ private static <T> T ensureType(Class<T> cls, Object v) throws Exception {
+ if (v == null || !cls.isAssignableFrom(v.getClass())) {
+ throw new Exception(String.format("Wrong value type '%s', expected '%s'.",
+ v == null ? "null" : v.getClass().getName(), cls.getName()));
+ }
+ return cls.cast(v);
+ }
+
+ private static <T> T checkType(Class<T> cls, Object v) {
+ return (v != null && cls.isAssignableFrom(v.getClass())) ? cls.cast(v) : null;
+ }
+
+ private ResultSet emptyResultSet(List<String> colNames) {
+ return new SQLNullResultSet((JDBCBridge.mock(colNames, Collections.<List<Object>>emptyList())));
+ }
+
}
diff --git a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
index 21e3f53..3df1aa4 100644
--- a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
+++ b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java
@@ -32,9 +32,6 @@ public abstract class AbstractJdbcIT {
private static String URL = String.format("tarantool://%s:%d?user=%s&password=%s", host, port, user, pass);
private static String[] initSql = new String[] {
- "DROP TABLE IF EXISTS test",
- "DROP TABLE IF EXISTS test_types",
-
"CREATE TABLE test(id INT PRIMARY KEY, val VARCHAR(100))",
"INSERT INTO test VALUES (1, 'one'), (2, 'two'), (3, 'three')",
@@ -78,12 +75,15 @@ public abstract class AbstractJdbcIT {
"X'010203040506'," + //LONGVARBINARY
"'1983-03-14'," + //DATE
"'12:01:06'," + //TIME
- "129479994)" //TIMESTAMP
+ "129479994)", //TIMESTAMP
+
+ "CREATE TABLE test_compound(id1 INT, id2 INT, val VARCHAR(100), PRIMARY KEY (id2, id1))"
};
private static String[] cleanSql = new String[] {
"DROP TABLE IF EXISTS test",
- "DROP TABLE IF EXISTS test_types"
+ "DROP TABLE IF EXISTS test_types",
+ "DROP TABLE IF EXISTS test_compound"
};
static Object[] testRow = new Object[] {
@@ -112,6 +112,7 @@ public abstract class AbstractJdbcIT {
@BeforeAll
public static void setupEnv() throws Exception {
+ sqlExec(cleanSql);
sqlExec(initSql);
}
diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
index b934bbe..17ab086 100644
--- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
+++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java
@@ -1,11 +1,11 @@
package org.tarantool.jdbc;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.BeforeEach;
import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -18,7 +18,7 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
private DatabaseMetaData meta;
@BeforeEach
- public void setUp() throws Exception {
+ public void setUp() throws SQLException {
meta = conn.getMetaData();
}
@@ -45,6 +45,9 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
assertTrue(rs.next());
assertEquals("TEST_TYPES", rs.getString("TABLE_NAME"));
+ assertTrue(rs.next());
+ assertEquals("TEST_COMPOUND", rs.getString("TABLE_NAME"));
+
assertFalse(rs.next());
rs.close();
@@ -84,7 +87,6 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
rs.close();
}
- @Disabled(value="Test ignored, issue#41")
@Test
public void testGetPrimaryKeys() throws SQLException {
ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
@@ -92,15 +94,93 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT {
assertNotNull(rs);
assertTrue(rs.next());
- assertNull(rs.getString("TABLE_CAT"));
- assertNull(rs.getString("TABLE_SCHEM"));
- assertEquals("TEST", rs.getString("TABLE_NAME"));
- assertEquals("ID", rs.getString("COLUMN_NAME"));
- assertEquals(1, rs.getInt("KEY_SEQ"));
- assertEquals("pk_unnamed_TEST_1", rs.getString("PK_NAME"));
+ checkGetPrimaryKeysRow(rs, "TEST", "ID", "pk_unnamed_TEST_1", 1);
+
+ assertFalse(rs.next());
+
+ rs.close();
+ }
+
+ @Test
+ public void testGetPrimaryKeysCompound() throws SQLException {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "TEST_COMPOUND");
+
+ assertNotNull(rs);
+ assertTrue(rs.next());
+ checkGetPrimaryKeysRow(rs, "TEST_COMPOUND", "ID1", "pk_unnamed_TEST_COMPOUND_1", 2);
+
+ assertTrue(rs.next());
+ checkGetPrimaryKeysRow(rs, "TEST_COMPOUND", "ID2", "pk_unnamed_TEST_COMPOUND_1", 1);
+
+ assertFalse(rs.next());
+ rs.close();
+ }
+
+ @Test
+ public void testGetPrimaryKeysIgnoresCatalogSchema() throws SQLException {
+ String[] vals = new String[] {null, "", "IGNORE"};
+ for (String cat : vals) {
+ for (String schema : vals) {
+ ResultSet rs = meta.getPrimaryKeys(cat, schema, "TEST");
+
+ assertNotNull(rs);
+ assertTrue(rs.next());
+ checkGetPrimaryKeysRow(rs, "TEST", "ID", "pk_unnamed_TEST_1", 1);
+ assertFalse(rs.next());
+ rs.close();
+ }
+ }
+ }
+
+ @Test
+ public void testGetPrimaryKeysNotFound() throws SQLException {
+ String[] tables = new String[] {null, "", "NOSUCHTABLE"};
+ for (String t : tables) {
+ ResultSet rs = meta.getPrimaryKeys(null, null, t);
+ assertNotNull(rs);
+ assertFalse(rs.next());
+ rs.close();
+ }
+ }
+
+ @Test
+ public void testGetPrimaryKeyNonSQLSpace() throws SQLException {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "_vspace");
+ assertNotNull(rs);
assertFalse(rs.next());
+ rs.close();
+ }
+ @Test
+ public void testGetPrimaryKeysOrderOfColumns() throws SQLException {
+ ResultSet rs = meta.getPrimaryKeys(null, null, "TEST");
+ assertNotNull(rs);
+ ResultSetMetaData rsMeta = rs.getMetaData();
+ assertEquals(6, rsMeta.getColumnCount());
+ assertEquals("TABLE_CAT", rsMeta.getColumnName(1));
+ assertEquals("TABLE_SCHEM", rsMeta.getColumnName(2));
+ assertEquals("TABLE_NAME", rsMeta.getColumnName(3));
+ assertEquals("COLUMN_NAME", rsMeta.getColumnName(4));
+ assertEquals("KEY_SEQ", rsMeta.getColumnName(5));
+ assertEquals("PK_NAME", rsMeta.getColumnName(6));
rs.close();
}
+
+ private void checkGetPrimaryKeysRow(ResultSet rs, String table, String colName, String pkName, int seq)
+ throws SQLException {
+ assertNull(rs.getString("TABLE_CAT"));
+ assertNull(rs.getString("TABLE_SCHEM"));
+ assertEquals(table, rs.getString("TABLE_NAME"));
+ assertEquals(colName, rs.getString("COLUMN_NAME"));
+ assertEquals(seq, rs.getInt("KEY_SEQ"));
+ assertEquals(pkName, rs.getString("PK_NAME"));
+
+ assertNull(rs.getString(1));
+ assertNull(rs.getString(2));
+ assertEquals(table, rs.getString(3));
+ assertEquals(colName, rs.getString(4));
+ assertEquals(seq, rs.getInt(5));
+ assertEquals(pkName, rs.getString(6));
+ }
}
diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
new file mode 100644
index 0000000..39d6326
--- /dev/null
+++ b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
@@ -0,0 +1,60 @@
+package org.tarantool.jdbc;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
+import org.tarantool.TarantoolConnection;
+
+import java.sql.DatabaseMetaData;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Properties;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.FORMAT_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.INDEX_FORMAT_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACE_ID_IDX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata._VINDEX;
+import static org.tarantool.jdbc.SQLDatabaseMetadata._VSPACE;
+
+public class JdbcExceptionHandlingTest {
+ /**
+ * Simulates meta parsing error: missing "name" field in a space format for the primary key.
+ *
+ * @throws Exception on failure.
+ */
+ @Test
+ public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws Exception {
+ TarantoolConnection tntCon = mock(TarantoolConnection.class);
+ SQLConnection conn = new SQLConnection(tntCon, "", new Properties());
+
+ Object[] spc = new Object[7];
+ spc[FORMAT_IDX] = Collections.singletonList(new HashMap<String, Object>());
+ spc[SPACE_ID_IDX] = 1000;
+
+ doReturn(Collections.singletonList(Arrays.asList(spc))).when(tntCon)
+ .select(_VSPACE, 2, Collections.singletonList("TEST"), 0, 1, 0);
+
+ Object[] idx = new Object[6];
+ idx[INDEX_FORMAT_IDX] = Collections.singletonList(
+ new HashMap<String, Object>() {{ put("field", 0);}});
+
+ doReturn(Collections.singletonList(Arrays.asList(idx))).when(tntCon)
+ .select(_VINDEX, 0, Arrays.asList(1000, 0), 0, 1, 0);
+
+ final DatabaseMetaData meta = conn.getMetaData();
+
+ Throwable t = assertThrows(SQLException.class, new Executable() {
+ @Override
+ public void execute() throws Throwable {
+ meta.getPrimaryKeys(null, null, "TEST");
+ }
+ }, "Error processing metadata for table \"TEST\".");
+
+ assertTrue(t.getCause().getMessage().contains("Wrong value type"));
+ }
+}
--
1.8.3.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-10-10 11:09 ` Sergei Kalashnikov
@ 2018-10-10 12:50 ` Alexander Turenko
2018-10-10 13:19 ` Sergei Kalashnikov
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Turenko @ 2018-10-10 12:50 UTC (permalink / raw)
To: Sergei Kalashnikov; +Cc: tarantool-patches
On Wed, Oct 10, 2018 at 02:09:48PM +0300, Sergei Kalashnikov wrote:
> Sure. Please find the amended patch at the very end of this mail.
>
Found one more such case. Sorry, missed that before.
diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
index 39d6326..8cc7acc 100644
--- a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
+++ b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
@@ -25,10 +25,10 @@ public class JdbcExceptionHandlingTest {
/**
* Simulates meta parsing error: missing "name" field in a space format for the primary key.
*
- * @throws Exception on failure.
+ * @throws SQLException on failure.
*/
@Test
- public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws Exception {
+ public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws SQLException {
TarantoolConnection tntCon = mock(TarantoolConnection.class);
SQLConnection conn = new SQLConnection(tntCon, "", new Properties());
You can don't attach the whole patch in case of such small code tweaks
(at least for me), because we anyway kept in sync using the repository.
WBR, Alexander Turenko.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-10-10 12:50 ` Alexander Turenko
@ 2018-10-10 13:19 ` Sergei Kalashnikov
2018-10-10 13:40 ` Alexander Turenko
0 siblings, 1 reply; 11+ messages in thread
From: Sergei Kalashnikov @ 2018-10-10 13:19 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
No problem. I pushed the amended commit to the branch.
Thank you.
On Wed, Oct 10, 2018 at 03:50:20PM +0300, Alexander Turenko wrote:
> On Wed, Oct 10, 2018 at 02:09:48PM +0300, Sergei Kalashnikov wrote:
> > Sure. Please find the amended patch at the very end of this mail.
> >
>
> Found one more such case. Sorry, missed that before.
>
> diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
> index 39d6326..8cc7acc 100644
> --- a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
> +++ b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
> @@ -25,10 +25,10 @@ public class JdbcExceptionHandlingTest {
> /**
> * Simulates meta parsing error: missing "name" field in a space format for the primary key.
> *
> - * @throws Exception on failure.
> + * @throws SQLException on failure.
> */
> @Test
> - public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws Exception {
> + public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws SQLException {
> TarantoolConnection tntCon = mock(TarantoolConnection.class);
> SQLConnection conn = new SQLConnection(tntCon, "", new Properties());
>
> You can don't attach the whole patch in case of such small code tweaks
> (at least for me), because we anyway kept in sync using the repository.
>
> WBR, Alexander Turenko.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: fix primary keys meta retrieval
2018-10-10 13:19 ` Sergei Kalashnikov
@ 2018-10-10 13:40 ` Alexander Turenko
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Turenko @ 2018-10-10 13:40 UTC (permalink / raw)
To: Sergei Kalashnikov; +Cc: tarantool-patches
Thanks!
I have checked in the patch into connector-1.8.jdbc branch.
WBR, Alexander Turenko.
On Wed, Oct 10, 2018 at 04:19:09PM +0300, Sergei Kalashnikov wrote:
> No problem. I pushed the amended commit to the branch.
>
> Thank you.
>
> On Wed, Oct 10, 2018 at 03:50:20PM +0300, Alexander Turenko wrote:
> > On Wed, Oct 10, 2018 at 02:09:48PM +0300, Sergei Kalashnikov wrote:
> > > Sure. Please find the amended patch at the very end of this mail.
> > >
> >
> > Found one more such case. Sorry, missed that before.
> >
> > diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
> > index 39d6326..8cc7acc 100644
> > --- a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
> > +++ b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java
> > @@ -25,10 +25,10 @@ public class JdbcExceptionHandlingTest {
> > /**
> > * Simulates meta parsing error: missing "name" field in a space format for the primary key.
> > *
> > - * @throws Exception on failure.
> > + * @throws SQLException on failure.
> > */
> > @Test
> > - public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws Exception {
> > + public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws SQLException {
> > TarantoolConnection tntCon = mock(TarantoolConnection.class);
> > SQLConnection conn = new SQLConnection(tntCon, "", new Properties());
> >
> > You can don't attach the whole patch in case of such small code tweaks
> > (at least for me), because we anyway kept in sync using the repository.
> >
> > WBR, Alexander Turenko.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-10-10 13:40 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 9:33 [tarantool-patches] [PATCH] jdbc: fix primary keys meta retrieval Sergei Kalashnikov
2018-10-01 13:04 ` [tarantool-patches] " Alexander Turenko
2018-10-02 17:09 ` Sergei Kalashnikov
2018-10-09 9:21 ` Sergei Kalashnikov
2018-10-09 15:40 ` Alexander Turenko
2018-10-10 8:14 ` Sergei Kalashnikov
2018-10-10 10:19 ` Alexander Turenko
2018-10-10 11:09 ` Sergei Kalashnikov
2018-10-10 12:50 ` Alexander Turenko
2018-10-10 13:19 ` Sergei Kalashnikov
2018-10-10 13:40 ` Alexander Turenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox