crate / crate-jdbc

A JDBC driver for CrateDB.
https://crate.io/docs/jdbc/
Apache License 2.0
23 stars 29 forks source link

Many DatabaseMetaData methods returns a ResultSet with null elements #216

Closed rogerbj closed 7 years ago

rogerbj commented 7 years ago

While testing crate and the JDBC driver with our product DbVisualizer, many DatabaseMetaData methods returns a single row ResultSet with all column values set to null. This brakes many features in DbVisualizer. Would be better if these throws SQLFeatureNotSupportedException.

joemoe commented 7 years ago

hi @rogerbj, thanks for your feedback. this is a good suggestion. we'll discuss this. do you have a list methods that are called?

rogerbj commented 7 years ago

Hi @joemoe, thanks for the speedy reply. Basically all methods now returning ResultSet with a single row having all elements = null. Would be much better if these either throw SQLFeatureNotSupportedException (recommended) or null. We've tested numerous drivers and the way this is handled in create-jdbc is not taken care of in DbVisualizer (and probably other tools). We see an increased number of users asking for crate and it would be great if this can be resolved in the driver rather than us tweaking DbVisualizer.

joemoe commented 7 years ago

Ok, I need to check the reasons why this was implemented in this way and if we break something in other areas if we'd change the return values or throw errors. Would be really cool to have CrateDB support in DbVisualizer.

memcmp commented 7 years ago

Hi @rogerbj, thanks for reporting this issue. According to the documentation of DatabaseMetaData ( https://docs.oracle.com/javase/7/docs/api/java/sql/DatabaseMetaData.html ) an empty result set should be returned if a given form of metadata is not supported.

I think the problem in our jdbc driver is that we return a ResultSet which contains a row where every value is null instead of no rows at all. With this fix: https://github.com/crate/crate-jdbc/pull/217 the empty result set will return zero rows, that should solve the problem.

I would be really interested in your feedback and if you also think that this will solve the problem i'll do a release immediately.

rogerbj commented 7 years ago

Great! We've tried numerous drivers and due to the lack of exact details in JDBC what a driver should do when a specific method is not supported by the target database, the way this is handled varies a lot. The most common is to "return null", secondly is throw new SQLException("Method not implemented/supported"). The problem with returning an empty ResultSet is that it doesn't help determine whether a feature is supported or not, it may just be empty at the time when called. I thinnk you can stick with the current empty result set solution and we'll verify. You may verify as well if you are interested. Just install, connect, and click around and see what happens. Download DbVisualizer.

memcmp commented 7 years ago

Hi @rogerbj, i just release crate 2.1.7, which contains the fix. (https://bintray.com/crate/crate/crate-jdbc/2.1.7)

rogerbj commented 7 years ago

Works a lot better. Thanks!

joemoe commented 7 years ago

@rogerbj what does a lot better mean? Is there still something we should fix on in our JDBC driver? we'd really like to make DBVisualizer work with CrateDB.

rogerbj commented 7 years ago

Sorry, I meant it now works fine as far as my brief testings shows.

There are some DatabaseMetaData accessor methods that fail currently not used by DbVisualizer: getMaxCatalogNameLength = ERROR: TableUnknownException: Table 'pg_catalog.pg_namespace' unknown getMaxColumnNameLength = ERROR: TableUnknownException: Table 'pg_catalog.pg_namespace' unknown getMaxColumnsInIndex = ERROR: TableUnknownException: Table 'pg_catalog.pg_settings' unknown getMaxCursorNameLength = ERROR: TableUnknownException: Table 'pg_catalog.pg_namespace' unknown getMaxProcedureNameLength = ERROR: TableUnknownException: Table 'pg_catalog.pg_namespace' unknown getMaxSchemaNameLength = ERROR: TableUnknownException: Table 'pg_catalog.pg_namespace' unknown getMaxTableNameLength = ERROR: TableUnknownException: Table 'pg_catalog.pg_namespace' unknown getMaxUserNameLength = ERROR: TableUnknownException: Table 'pg_catalog.pg_namespace' unknown

The driver is also identifying itself as being: PostgreSQL Native Driver PostgreSQL 9.4.jre7

Would be more helpful if it listed relevant information such as: Crate JDBC Driver 2.1.7

joemoe commented 7 years ago

ok, thanks. we'll check this.