Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CALCITE-5964] Add test case checking that connection.getMetaData().getTables() handles responses with different signature shapes. #227

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[CALCITE-5964] Add test case checking that connection.getMetaData().getTables() handles responses with different signature shapes. #227

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b75a97f
[CALCITE-5964] Add test case checking that connection.getMetaData().g…
olivrlee Jul 28, 2023
d3e6a9f
Update MockProtobufService to pass my RemoteDriverMockTest
olivrlee Sep 11, 2023
af4e00b
Remove Type.CLASS changes
olivrlee Sep 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions core/src/main/java/org/apache/calcite/avatica/ConnectionConfigImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,19 @@ public <T> T getPlugin(Class<T> pluginClass, String defaultClassName,
return get_(pluginConverter(pluginClass, defaultInstance),
defaultClassName);
}

/** Returns the class value of this property. Throws if not set and no
* default. */
public Class<?> getClazz(Class<?> clazz, Class<?> defaultValue) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Java convention uses clazz because class is a keyword. getClass is not a keyword. Just call this getClass. Same goes for all other names that contain the word "class" but are not keywords.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a small chance for confusion with Object.getClass(). So, I would recommend calling it getType.

return getClazz(clazz, property.defaultValue().toString(), defaultValue); }

/** Returns the class value of this property. Throws if not set and no
* default. */
public Class<?> getClazz(Class<?> clazz, String defaultClassName,
Class<?> defaultValue) {
assert property.type() == ConnectionProperty.Type.CLASS;
return get_(clazzConverter(clazz, defaultValue), defaultClassName);
}
}

/** Callback to parse a property from string to its native type.
Expand Down Expand Up @@ -434,6 +447,30 @@ public T apply(ConnectionProperty connectionProperty, String s) {
}
};
}

public static <T> Converter<T> clazzConverter(final T clazz,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T can be narrowed to Class<T>, since there's only 1 callsite for this function and it will always pass in a Class<X> for some X. That would also make the variable name less confusing.

I suppose that would make this a Converter<Class<?>>. You may want to add another type variable for defaultClass.

final T defaultClass) {
return new Converter<T>() {
public T apply(ConnectionProperty connectionProperty, String s) {
if (s == null) {
if (defaultClass != null) {
return defaultClass;
}
if (!connectionProperty.required()) {
return null;
}
throw new RuntimeException("Required property '"
+ connectionProperty.camelName() + "' not specified");
}
try {
return (T) Class.forName(s);
} catch (ClassNotFoundException e) {
return defaultClass;
}

}
};
}
}

// End ConnectionConfigImpl.java
7 changes: 6 additions & 1 deletion core/src/main/java/org/apache/calcite/avatica/ConnectionProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ enum Type {
STRING,
NUMBER,
ENUM,
PLUGIN;
PLUGIN,
CLASS;

/** Deduces the class of a property of this type, given the default value
* and the user-specified value class (each of which may be null, unless
Expand Down Expand Up @@ -94,6 +95,9 @@ public boolean valid(Object defaultValue, Class clazz) {
case ENUM:
return Enum.class.isAssignableFrom(clazz)
&& (defaultValue == null || clazz.isInstance(defaultValue));
case CLASS:
return clazz == Class.class
&& (defaultValue == null || defaultValue instanceof Class);
default:
throw new AssertionError();
}
Expand All @@ -107,6 +111,7 @@ public Class defaultValueClass() {
return Number.class;
case STRING:
return String.class;
case CLASS:
case PLUGIN:
return Object.class;
default:
Expand Down
40 changes: 39 additions & 1 deletion core/src/main/java/org/apache/calcite/avatica/MetaImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,32 @@ public MetaColumn(
public String getName() {
return columnName;
}
public static String[] getColumnNames() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might as well link to some documentation: https://docs.oracle.com/en/java/javase/20/docs/api/java.sql/java/sql/DatabaseMetaData.html#getColumns(java.lang.String,java.lang.String,java.lang.String,java.lang.String)

Also add a comment explaining why anybody might want to override this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's static so it can't be overridden, the reader thinks. The fact that the caller is using reflection to find and invoke it could definitely do with some commentary, if that is indeed the best way for the caller to get this information.

olivrlee marked this conversation as resolved.
Show resolved Hide resolved
return Arrays.asList("TABLE_CAT",
"TABLE_SCHEM",
"TABLE_NAME",
"COLUMN_NAME",
"DATA_TYPE",
"TYPE_NAME",
"COLUMN_SIZE",
"BUFFER_LENGTH",
"DECIMAL_DIGITS",
"NUM_PREC_RADIX",
"NULLABLE",
"REMARKS",
"COLUMN_DEF",
"SQL_DATA_TYPE",
"SQL_DATETIME_SUB",
"CHAR_OCTET_LENGTH",
"ORDINAL_POSITION",
"IS_NULLABLE",
"SCOPE_CATALOG",
"SCOPE_SCHEMA",
"SCOPE_TABLE",
"SOURCE_DATA_TYPE",
"IS_AUTOINCREMENT",
"IS_GENERATEDCOLUMN").toArray(new String[0]);
}
}

/** Metadata describing a table. */
Expand Down Expand Up @@ -419,8 +445,20 @@ public MetaTable(
public String getName() {
return tableName;
}
}

public static String[] getColumnNames() {
return Arrays.asList("TABLE_CAT",
"TABLE_SCHEM",
"TABLE_NAME",
"TABLE_TYPE",
"REMARKS",
"TYPE_CAT",
"TYPE_SCHEM",
"TYPE_NAME",
"SELF_REFERENCING_COL_NAME",
"REF_GENERATION").toArray(new String[0]);
}
}
/** Metadata describing a schema. */
public static class MetaSchema implements Named {
@ColumnNoNulls
Expand Down
76 changes: 76 additions & 0 deletions core/src/main/java/org/apache/calcite/avatica/remote/MockJsonService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,82 @@ public Service create(AvaticaConnection connection) {
+ " \"parameters\": [],\n"
+ " \"cursorFactory\": {\"style\": \"ARRAY\"}\n"
+ "}}");
map1.put(
JsonService.encode(new TablesRequest(
connectionId, null, null, "additionalColumnsTest", Arrays.<String>asList())),
"{\"response\":\"resultSet\",\"connectionId\":\"" + connectionId + "\",\"statementId\":0,\"ownStatement\":true,"
+ "\"signature\":{\"columns\":["
+ "{\"ordinal\":0,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":0,\"signed\":false,"
+ " \"displaySize\":128,\"label\":\"TABLE_CAT\",\"columnName\":\"TABLE_CAT\",\"schemaName\":\"INFORMATION_SCHEMA\""
+ ",\"precision\":128,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\",\"catalogName\":\"PUBLIC\",\"type\":"
+ "{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,\""
+ "definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":1,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":0,\"signed\":false,"
+ "\"displaySize\":128,\"label\":\"TABLE_SCHEM\",\"columnName\":\"TABLE_SCHEM\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":128,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":2,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":0,\"signed\":false,"
+ "\"displaySize\":128,\"label\":\"TABLE_NAME\",\"columnName\":\"TABLE_NAME\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":128,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":3,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":0,\"signed\":false,"
+ "\"displaySize\":65536,\"label\":\"TABLE_TYPE\",\"columnName\":\"TABLE_TYPE\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":65536,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":4,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":1,\"signed\":false,"
+ "\"displaySize\":65536,\"label\":\"REMARKS\",\"columnName\":\"REMARKS\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":65536,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":5,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":1,\"signed\":false,"
+ "\"displaySize\":128,\"label\":\"TYPE_CAT\",\"columnName\":\"TYPE_CAT\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":128,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":6,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":1,\"signed\":false,"
+ "\"displaySize\":128,\"label\":\"TYPE_SCHEM\",\"columnName\":\"TYPE_SCHEM\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":128,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":7,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":1,\"signed\":false,"
+ "\"displaySize\":128,\"label\":\"TYPE_NAME\",\"columnName\":\"TYPE_NAME\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":128,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":8,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":1,\"signed\":false,"
+ "\"displaySize\":128,\"label\":\"SELF_REFERENCING_COL_NAME\",\"columnName\":\"SELF_REFERENCING_COL_NAME\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":128,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":9,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":1,\"signed\":false,"
+ "\"displaySize\":65536,\"label\":\"REF_GENERATION\",\"columnName\":\"REF_GENERATION\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":65536,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":10,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":1,\"signed\":false,"
+ "\"displaySize\":128,\"label\":\"HSQLDB_TYPE\",\"columnName\":\"HSQLDB_TYPE\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":128,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\":11,\"autoIncrement\":false,\"caseSensitive\":false,\"searchable\":true,\"currency\":false,\"nullable\":1,\"signed\":false,"
+ "\"displaySize\":5,\"label\":\"READ_ONLY\",\"columnName\":\"READ_ONLY\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":0,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":16,\"name\":\"BOOLEAN\",\"rep\":\"PRIMITIVE_BOOLEAN\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.Boolean\"},"
+ "{\"ordinal\":12,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":true,\"currency\":false,\"nullable\":1,\"signed\":false,"
+ "\"displaySize\":65536,\"label\":\"COMMIT_ACTION\",\"columnName\":\"COMMIT_ACTION\",\"schemaName\":\"INFORMATION_SCHEMA\",\"precision\":65536,\"scale\":0,\"tableName\":\"SYSTEM_TABLES\","
+ "\"catalogName\":\"PUBLIC\",\"type\":{\"type\":\"scalar\",\"id\":12,\"name\":\"VARCHAR\",\"rep\":\"STRING\"},\"readOnly\":true,\"writable\":false,"
+ "\"definitelyWritable\":false,\"columnClassName\":\"java.lang.String\"},"
+ "{\"ordinal\": 13,\"autoIncrement\": false,\"caseSensitive\": true,\"searchable\": true,\"currency\": false,\"nullable\": 1,\"signed\": false,"
+ "\"displaySize\": 65536,\"label\": \"EXTRA_LABEL\",\"columnName\": \"EXTRA_LABEL\",\"schemaName\": \"INFORMATION_SCHEMA\",\"precision\": 65536,\"scale\": 0,\"tableName\": \"SYSTEM_TABLES\","
+ "\"catalogName\": \"PUBLIC\",\"type\":{\"type\": \"scalar\",\"id\": 12,\"name\": \"VARCHAR\",\"rep\": \"STRING\"},\"readOnly\": true,\"writable\": false,"
+ "\"definitelyWritable\": false,\"columnClassName\": \"java.lang.String\"}],\"sql\":null,"
+ "\"parameters\":[],"
+ "\"cursorFactory\":{\"style\":\"LIST\",\"clazz\":null,\"fieldNames\":null},\"statementType\":null},"
+ "\"firstFrame\":{\"offset\":0,\"done\":true,"
+ "\"rows\":["
+ "[\"PUBLIC\",\"SCOTT\",\"DEPT\",\"TABLE\",null,null,null,null,null,null,\"MEMORY\",false,null,\"EXTRA_LABEL1\"],"
+ "[\"PUBLIC\",\"SCOTT\",\"EMP\",\"TABLE\",null,null,null,null,null,null,\"MEMORY\",false,null,\"EXTRA_LABEL2\"],"
+ "[\"PUBLIC\",\"SCOTT\",\"BONUS\",\"TABLE\",null,null,null,null,null,null,\"MEMORY\",false,null,\"EXTRA_LABEL3\"],"
+ "[\"PUBLIC\",\"SCOTT\",\"SALGRADE\",\"TABLE\",null,null,null,null,null,null,\"MEMORY\",false,null,null]]},"
+ "\"updateCount\":-1,\"rpcMetadata\":null}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This JSON blob is impenetrable. How did you produce this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran another test within Avatica and took that json blob and modified it
What does 'impenetrable' mean here?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard to read this data fixture and understand its direct relationship with assertions in tests.

There's also the usual problem with mocking - how do you keep your mocks in sync with the real implementation. When Avatica changes its response, tests depending on this fixture will go stale without further notice and there is no forcing function to cause them to be updated until or unless a regression occurs. But this problem is hard to solve without designing for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So one thing to note is that this test really just shows that whatever Signature comes in (different columns, different orders), Avatica is able to handle the different shapes and read them in the supplied order.
It doesn't test whether the real implementation is providing a correct response in the right order. That part should be up to the implementation.

Maybe that point wasn't clear by this additional test, perhaps I should shorten the JSON to contain fewer columns and add a comment stating that the point is that the signature column order matches up with the resultset columns?

);
map1.put(
"{\"request\":\"closeStatement\",\"connectionId\":\"" + connectionId + "\",\"statementId\":0}",
"{\"response\":\"closeStatement\"}"
);
map1.put(
"{\"request\":\"getColumns\",\"connectionId\":\"" + connectionId + "\",\"catalog\":null,\"schemaPattern\":null,"
+ "\"tableNamePattern\":\"my_table\",\"columnNamePattern\":null}",
Expand Down
32 changes: 32 additions & 0 deletions core/src/main/java/org/apache/calcite/avatica/remote/MockProtobufService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,38 @@ private Map<Request, Response> createMapping() {
Meta.Frame.create(0, true,
Arrays.<Object>asList(new Object[] {new Object[]{"my_table", 10}})), -1, null));

mappings.put(
new TablesRequest(connectionId, null, null,
"additionalColumnsTest", Collections.<String>emptyList()),
// ownStatement=false just to avoid the extra close statement call.
new ResultSetResponse("0", 1, false,
Meta.Signature.create(
Arrays.<ColumnMetaData>asList(
MetaImpl.columnMetaData("TABLE_CAT", 0, String.class, true),
MetaImpl.columnMetaData("TABLE_SCHEM", 1, String.class, true),
MetaImpl.columnMetaData("TABLE_NAME", 2, String.class, true),
MetaImpl.columnMetaData("TABLE_TYPE", 3, String.class, true),
MetaImpl.columnMetaData("REMARKS", 4, String.class, true),
MetaImpl.columnMetaData("TYPE_CAT", 5, String.class, true),
MetaImpl.columnMetaData("TYPE_SCHEM", 6, String.class, true),
MetaImpl.columnMetaData("TYPE_NAME", 7, String.class, true),
MetaImpl.columnMetaData("SELF_REFERENCING_COL_NAME", 8, String.class, true),
MetaImpl.columnMetaData("REF_GENERATION", 9, String.class, true),
MetaImpl.columnMetaData("HSQLDB_TYPE", 10, String.class, true),
MetaImpl.columnMetaData("READ_ONLY", 11, Boolean.class, true),
MetaImpl.columnMetaData("COMMIT_ACTION", 12, String.class, true),
MetaImpl.columnMetaData("EXTRA_LABEL", 13, Boolean.class, true)),
null, null, Meta.CursorFactory.ARRAY, Meta.StatementType.SELECT),
Meta.Frame.create(0, true,
Arrays.<Object>asList(
new Object[] {"table_cat", "table_schem", "table_name", "table_type", "remarks",
"type_cat", "type_schem", "type_name", "self_referencing_col_name",
"ref_generation", "hsqldb_type", true, "commit_action", "extra_label1"},
new Object[] {"table_cat", "table_schem", "table_name", "table_type",
"remarks", "type_cat", "type_schem", "type_name", "self_referencing_col_name",
"ref_generation", "hsqldb_type", false, "commit_action", "extra_label2"}
)), -1, null));

return Collections.unmodifiableMap(mappings);
}

Expand Down
15 changes: 15 additions & 0 deletions server/src/test/java/org/apache/calcite/avatica/RemoteDriverMockTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,21 @@ private Connection getMockConnection() {
connection.close();
}

@Test public void testTablesAdditionalColumns() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced of the usefulness of this test. Which part of the production code is actually being exercised? Kinda looks like we're just stubbing a mock, then testing that we stubbed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It mocks getting the response, but it actually tests the client code in RemoteMeta that handles the response and parses the signature into the ResultSet.

final Connection connection = getMockConnection();
final ResultSet resultSet =
connection.getMetaData().getTables(null, null, "additionalColumnsTest", new String[0]);
assertTrue(resultSet.next());
final ResultSetMetaData metaData = resultSet.getMetaData();
assertTrue(metaData.getColumnCount() == 14);
assertEquals("TABLE_CAT", metaData.getColumnName(1));
assertEquals("TABLE_SCHEM", metaData.getColumnName(2));
assertEquals("TABLE_NAME", metaData.getColumnName(3));
assertEquals("EXTRA_LABEL", metaData.getColumnName(14));
resultSet.close();
connection.close();
}

@Ignore
@Test public void testNoFactory() throws Exception {
final Connection connection =
Expand Down