search

marklogic-community / ml-jdbc-driver

Making JDBC connections to MarkLogic SQL/ODBC Server
Other
2 stars 4 forks source link

PgDatabaseMetaData.getColumns sets the columnSize for VARCHAR to -4 #1

Closed awal11 closed 5 years ago

awal11 commented 5 years ago

I am trying to use the driver together with PrestoDB. Presto needs to analyze the metadata returned by ML jdbc driver and I encountered few issues, this would be the first of them.

The metadata for VARCHAR set the columnSize to value -4 - I think this is happening because:

  1. PgDatabaseMetaData line 1815: always returns 0 as attypemod
  2. PgDatabaseMetaData line 1837: uses this value as typeMod
  3. PgDatabaseMetaData line 1854: uses typeMod (set to 0) to determine length of this varchar using class TypeInfoCache
  4. TypeInfoCache line 623: is probably left unmodified from Postgres, not expecting 0, returns 0-4=-4 instead of a real value or _unknownLength that would probably behave better.
  5. Presto fails with some assertions when analyzing the query results, it does not know what to do with VARCHAR of length -4

To address this issue I would suggest either

  1. set the columnSize to real length of varchar, but I assume it is not possible with ODBC views in ML?
  2. set the columnSize to _unknownLength for varchars

I can create a pull request for the latter case for PgDatabaseMetaData class if you support this solution

awal11 commented 5 years ago

Probably the best approach would be to always set the atttypemod in all queries in PgDatabaseMetaData to -1 as this is a value expected by class TypeInfoCache.

bobstarbird commented 5 years ago

Awesome feedback! I think you are right that _unknownLength is the best solution since in fact is is unknown.

awal11 commented 5 years ago

Should I create pull requests for the issues I encounter or you plan any major refactoring to the code and it does not make bigger sense?

bobstarbird commented 5 years ago

Please take a look at the new CONTRIBUTING for the pull request

Agree to the contributor License

Before we can merge your changes, you need to sign a Contributor License Agreement. You only need to do this once. This license is for your protection as well as the protection of MarkLogic and its licensees. This Agreement does not otherwise change your rights to use your own Contributions for any other purpose.

bobstarbird commented 5 years ago

I agree this is the correct fix. -1 (vs 0). case Oid.BIT: in TypeInfoCache is a special case for later. The query should be "-1 as atttypmod" to match the typmod handling in TypeInfoCache. Please try to become a contributor. And thank you very much for continuing to move these issues forward.

awal11 commented 5 years ago

I have sent the CLA to the indicated email address On Tue, 5 Feb 2019 at 01:52, bobstarbird notifications@github.com wrote:

I agree this is the correct fix. -1 (vs 0). case Oid.BIT: in TypeInfoCache is a special case for later. The query should be "-1 as atttypmod" to match the typmod handling in TypeInfoCache. Please try to become a contributor.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/marklogic-community/ml-jdbc-driver/issues/1#issuecomment-460472921, or mute the thread https://github.com/notifications/unsubscribe-auth/AGQQxLA0dc0BRTAmu28qr7Jx_eYHxV9yks5vKNW4gaJpZM4Z3X3z .