java.jdbc

Fix PostgreSQL tests

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    Postgres 9.4.7

Description

Currently tests fail on Postgres. There are several reasons for this.

1. Clean-up fixture is broken because it tries multiple drop tables in a transaction. When a first drop table fails, due to missing table, the transaction is aborted. I doubt other DBs even support transactional DDL so I think clean-up could do without a transaction.

2. Multiple tests check the return value of db-do-prepared-return-keys. In Postgres this returns the complete rows instead of just ids. Tests should map results to retrieve just ids when using Postgres.

3. insert-one-col-val and insert-one-col-val-opts presume that Postgres returns the complete rows so it maps the results to retrieve just ids. For some reason in this case insert! only returns ids in Postgres. Probably something to do with this certain arity of insert!. I presume tests are correct but the implementation is broken.

I have some code ready to fix 1 and 2. If my assessments of the problems and solutions sound good I can finalize those and create a patch.

P.S. I think it would be important to eventually run Postgres tests on CI.

Activity

Hide
Juho Teperi added a comment -

This patch fixes problem 2.

I ddidn't include fix for 1. yet as I'm not sure what is the best solution for it.

Show
Juho Teperi added a comment - This patch fixes problem 2. I ddidn't include fix for 1. yet as I'm not sure what is the best solution for it.
Hide
Sean Corfield added a comment - - edited

I'm adding a select-key function that maps results for all DBs – it's identity for most DBs and the :id function for PostgreSQL – and then calling this on all inserted results (or map ping it over the results). This removes the conditional (if (postgres? db) ...) all over the place – which was inserted as a patch on the original set of tests a long time ago. Now I have a consistent structure to use in future tests.

For 1. I'm just removing the transaction wrapper: DDL just shouldn't be transactional.

I'll take a look at 3. in more detail once I have the tests passing with select-key applied.

Show
Sean Corfield added a comment - - edited I'm adding a select-key function that maps results for all DBs – it's identity for most DBs and the :id function for PostgreSQL – and then calling this on all inserted results (or map ping it over the results). This removes the conditional (if (postgres? db) ...) all over the place – which was inserted as a patch on the original set of tests a long time ago. Now I have a consistent structure to use in future tests. For 1. I'm just removing the transaction wrapper: DDL just shouldn't be transactional. I'll take a look at 3. in more detail once I have the tests passing with select-key applied.
Hide
Sean Corfield added a comment - - edited

The behavior in 3. is correct, but not very helpful.

Insert row-as-map (both insert! and insert-multi!) will insert each row separately and ask the JDBC driver to return the generated keys (PostgreSQL returns the whole row... very helpful!).

Insert row-as-col-vals (both insert! and insert-multi!) will perform a single insert operation using executeBatch and then call getUpdateCount which returns just a series of 1's – since the SQL batch operation updated one row for each item in the batch.

I've updated the docstrings in both insert! and insert-multi! to make this clearer.

That is done because it didn't seem possible to portably (across all supported databases) insert multiple rows with a single statement, containing all the values. I can revisit this if someone believes it is possible.

Show
Sean Corfield added a comment - - edited The behavior in 3. is correct, but not very helpful. Insert row-as-map (both insert! and insert-multi!) will insert each row separately and ask the JDBC driver to return the generated keys (PostgreSQL returns the whole row... very helpful!). Insert row-as-col-vals (both insert! and insert-multi!) will perform a single insert operation using executeBatch and then call getUpdateCount which returns just a series of 1's – since the SQL batch operation updated one row for each item in the batch. I've updated the docstrings in both insert! and insert-multi! to make this clearer. That is done because it didn't seem possible to portably (across all supported databases) insert multiple rows with a single statement, containing all the values. I can revisit this if someone believes it is possible.
Hide
Sean Corfield added a comment -

Fixed in Git (aside from the docstring changes, this is purely a test code update).

Show
Sean Corfield added a comment - Fixed in Git (aside from the docstring changes, this is purely a test code update).

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: