OpenJPA
  1. OpenJPA
  2. OPENJPA-557

Primary key sequences broken with postgres schemas

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.2
    • Fix Version/s: 1.3.0, 2.0.0-M2
    • Component/s: jdbc
    • Labels:
      None

      Description

      as per http://www.nabble.com/forum/ViewPost.jtp?post=15460899&framed=y

      OpenJPA issues a SELECT currval('user_id_seq'); query to get the current PK value on postgres. This should not execute correctly when using a schema. The correct query is SELECT currval('schemaname.user_id_seq');

      1. OPENJPA-557.patch
        14 kB
        Milosz Tylenda

        Issue Links

          Activity

          Hide
          Catalina Wei added a comment -

          Patch provided by Milosz Tylenda has been checked in under svn trunk r729180 and branch 1.3.x r729181.

          Show
          Catalina Wei added a comment - Patch provided by Milosz Tylenda has been checked in under svn trunk r729180 and branch 1.3.x r729181.
          Hide
          Milosz Tylenda added a comment -

          Applying the patch and running the test suite reveals another problem with schema handling in PostgreSQL. I will open a separate JIRA issue.

          Show
          Milosz Tylenda added a comment - Applying the patch and running the test suite reveals another problem with schema handling in PostgreSQL. I will open a separate JIRA issue.
          Hide
          Milosz Tylenda added a comment -

          The patch attached fixes the problem as suggested by Mehmet in OPENJPA-582:

          lastGeneratedKeyQuery = "SELECT CURRVAL(''

          {1}

          _

          {0}

          _seq'')";

          This reflects well how PostgreSQL is creating implicit sequences for SERIAL (auto-increment) columns [1]. Lower casing is already handled in DBDictionary.convertSchemaCase(String) method.

          I have updated the TestMultipleSchemaNames test case to make the issue visible. Previously the test case was failing in the beginning on PostgreSQL because of schema handling. Derby and DB2 create schema automatically in CREATE TABLE if schema does not exist. PostgreSQL requires explicit schema creation. I have added this to the test case. If we run the test case on Oracle or MySQL, the things are even worse because Oracle treats what we call schema a user name and MySQL treats it as database name.

          I added two DogX classes and removed comments from some others as I found them misleading.

          [1] http://www.postgresql.org/docs/8.3/interactive/datatype-numeric.html#DATATYPE-SERIAL

          Show
          Milosz Tylenda added a comment - The patch attached fixes the problem as suggested by Mehmet in OPENJPA-582 : lastGeneratedKeyQuery = "SELECT CURRVAL('' {1} _ {0} _seq'')"; This reflects well how PostgreSQL is creating implicit sequences for SERIAL (auto-increment) columns [1] . Lower casing is already handled in DBDictionary.convertSchemaCase(String) method. I have updated the TestMultipleSchemaNames test case to make the issue visible. Previously the test case was failing in the beginning on PostgreSQL because of schema handling. Derby and DB2 create schema automatically in CREATE TABLE if schema does not exist. PostgreSQL requires explicit schema creation. I have added this to the test case. If we run the test case on Oracle or MySQL, the things are even worse because Oracle treats what we call schema a user name and MySQL treats it as database name. I added two DogX classes and removed comments from some others as I found them misleading. [1] http://www.postgresql.org/docs/8.3/interactive/datatype-numeric.html#DATATYPE-SERIAL
          Hide
          Michal Borowiecki added a comment -

          I experienced this bug in geronimo 2.1.3

          I checked out openjpa tag 1.0.3 sources, applied the patch, compiled and substituted the class file in openjpa-1.0.3.jar in geronimo 2.1.3 repository.
          I confirm the patch works.

          This issue is very important and I hope someone applies it to trunk soon

          Thanks for the patch Roger!

          Show
          Michal Borowiecki added a comment - I experienced this bug in geronimo 2.1.3 I checked out openjpa tag 1.0.3 sources, applied the patch, compiled and substituted the class file in openjpa-1.0.3.jar in geronimo 2.1.3 repository. I confirm the patch works. This issue is very important and I hope someone applies it to trunk soon Thanks for the patch Roger!
          Hide
          Roger Keays added a comment -

          Here is my patch for OpenJPA 1.0.2. You might prefer to move the code to the superclass though.

          Index: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java
          ===================================================================
          — openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java (revision 641780)
          +++ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java (working copy)
          @@ -149,6 +149,21 @@
          "STORE", "VACUUM", "VERBOSE", "VERSION",
          }));
          }
          +
          + /**
          + * Prepend schema names to sequence names if there is one. This
          + * method does not escape reserved words in the schema name or
          + * sequence name.
          + */
          + protected String getGeneratedKeySequenceName(Column col) {
          + String sequence = super.getGeneratedKeySequenceName(col);
          + String schema = col.getSchemaName();
          + if (schema != null && schema.length() > 0)

          { + return schema + "." + sequence; + }

          else

          { + return sequence; + }

          + }

          public Date getDate(ResultSet rs, int column)
          throws SQLException {

          This patch has been fine in production for me over the last week.

          Adam Hardy also adds:

          You might want to make that schema.toLowerCase()

          Postgres diverges from the JDBC spec by making everything lower case and it
          won't find an upper case schema.

          Show
          Roger Keays added a comment - Here is my patch for OpenJPA 1.0.2. You might prefer to move the code to the superclass though. Index: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java =================================================================== — openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java (revision 641780) +++ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/PostgresDictionary.java (working copy) @@ -149,6 +149,21 @@ "STORE", "VACUUM", "VERBOSE", "VERSION", })); } + + /** + * Prepend schema names to sequence names if there is one. This + * method does not escape reserved words in the schema name or + * sequence name. + */ + protected String getGeneratedKeySequenceName(Column col) { + String sequence = super.getGeneratedKeySequenceName(col); + String schema = col.getSchemaName(); + if (schema != null && schema.length() > 0) { + return schema + "." + sequence; + } else { + return sequence; + } + } public Date getDate(ResultSet rs, int column) throws SQLException { This patch has been fine in production for me over the last week. Adam Hardy also adds: You might want to make that schema.toLowerCase() Postgres diverges from the JDBC spec by making everything lower case and it won't find an upper case schema.

            People

            • Assignee:
              Unassigned
              Reporter:
              Roger Keays
            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development