OpenJPA
  1. OpenJPA
  2. OPENJPA-736

Combine insert and select SQL together for generated Id strategy=GenerationType.IDENTITY

    Details

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

      Description

      Currently if the strategy of the generated id is GenerationType.IDENTITY, when an entity object is created, openjpa will generate two SQL statements. The following is an example running on DB2:

      (1) INSERT INTO EntityA (col1, col2, col3, version) VALUES (?, ?, ?, ?)
      [params=(int) 1, (int) 1, (int) 1, (int) 1]

      (2) SELECT IDENTITY_VAL_LOCAL() FROM SYSIBM.SYSDUMMY1

      A performance improvement is to take advantage of the "select from final table" feature in DB2 to combine the insert and select statement into a single SQL statement as shown below:

      SELECT id FROM FINAL TABLE (INSERT INTO EntityA (col1, col2, col3, version) VALUES (?, ?, ?, ?) )

      1. supportsGetGeneratedKeys.patch
        2 kB
        Milosz Tylenda
      2. openjpa-736.patch
        7 kB
        Fay Wang

        Issue Links

          Activity

          Donald Woods made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Milosz Tylenda added a comment -

          For information, Oracle's driver also supports the getGeneratedKeys although Oracle database does not support auto-increment columns. The generated keys retrieval seems to play nicely with our OracleDictionary.useTriggersForAutoAssign feature for emulating auto-increment columns.

          Show
          Milosz Tylenda added a comment - For information, Oracle's driver also supports the getGeneratedKeys although Oracle database does not support auto-increment columns. The generated keys retrieval seems to play nicely with our OracleDictionary.useTriggersForAutoAssign feature for emulating auto-increment columns.
          Milosz Tylenda made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Fix Version/s 2.0.0 [ 12313483 ]
          Resolution Fixed [ 1 ]
          Hide
          Milosz Tylenda added a comment -

          Change applied also to 1.3.x branch.

          Show
          Milosz Tylenda added a comment - Change applied also to 1.3.x branch.
          Hide
          Milosz Tylenda added a comment -

          Points 1, 2 and 3 applied to trunk.

          Show
          Milosz Tylenda added a comment - Points 1, 2 and 3 applied to trunk.
          Milosz Tylenda made changes -
          Attachment supportsGetGeneratedKeys.patch [ 12408877 ]
          Hide
          Milosz Tylenda added a comment -

          The attached patch supportsGetGeneratedKeys.patch addresses point 1. The idea is to auto-detect the property only if user did not specify the property value. In order to do that, I have changed the property type from primitive boolean to Boolean. Let me know if anybody sees this problematic.

          Show
          Milosz Tylenda added a comment - The attached patch supportsGetGeneratedKeys.patch addresses point 1. The idea is to auto-detect the property only if user did not specify the property value. In order to do that, I have changed the property type from primitive boolean to Boolean. Let me know if anybody sees this problematic.
          Hide
          Milosz Tylenda added a comment -

          I have restored the signatures. Let me know if a problem still exists.

          Show
          Milosz Tylenda added a comment - I have restored the signatures. Let me know if a problem still exists.
          Hide
          Milosz Tylenda added a comment -

          Albert, I can restore the signatures but unfortunately I am out of source access till the end of week. If it is more urgent, you could roll my change back.

          Sorry for the problem. My intent was to make signatures simpler - did not know someone might have already subclassed this.

          Show
          Milosz Tylenda added a comment - Albert, I can restore the signatures but unfortunately I am out of source access till the end of week. If it is more urgent, you could roll my change back. Sorry for the problem. My intent was to make signatures simpler - did not know someone might have already subclassed this.
          Hide
          Albert Lee added a comment -

          Signature change in PreparedStatementManagerImpl has broken subclass in (non-OpenJPA) derived product build.

          Albert Lee.

          Show
          Albert Lee added a comment - Signature change in PreparedStatementManagerImpl has broken subclass in (non-OpenJPA) derived product build. Albert Lee.
          Hide
          Milosz Tylenda added a comment -

          Now when I have finally got SQL Server 2008 at my database pasture (what a glorious day! , I have successfully tested the proposed solution with DB2, MySQL, PostgreSQL and SQL Server. I have committed the change to the trunk. I have made DBDictionary.convertSchemaCase public as the invocation is needed for PostgreSQL.

          A few more things to do:
          1. Currently the support for JDBC 3 generated keys retrieval is auto-detected. I think we should also allow users to enable/disable it explicitly. Some people may want the old behaviour (issuing a database-specific query to fetch generated keys) for some reasons. Also, if a bug is lurking somewhere, we have an easy workaround by disabling the feature.
          2. TestGenerationType could be extended to test both ways of generated keys retrieval where possible.
          3. The user manual might need an update.
          4. Apply the changes to 1.3.x.

          Show
          Milosz Tylenda added a comment - Now when I have finally got SQL Server 2008 at my database pasture (what a glorious day! , I have successfully tested the proposed solution with DB2, MySQL, PostgreSQL and SQL Server. I have committed the change to the trunk. I have made DBDictionary.convertSchemaCase public as the invocation is needed for PostgreSQL. A few more things to do: 1. Currently the support for JDBC 3 generated keys retrieval is auto-detected. I think we should also allow users to enable/disable it explicitly. Some people may want the old behaviour (issuing a database-specific query to fetch generated keys) for some reasons. Also, if a bug is lurking somewhere, we have an easy workaround by disabling the feature. 2. TestGenerationType could be extended to test both ways of generated keys retrieval where possible. 3. The user manual might need an update. 4. Apply the changes to 1.3.x.
          Hide
          Milosz Tylenda added a comment -

          Omitting an identity column is not the problem. The MySQL issue is that it does not allow the retrieval of generated column by its actual name - coulmn name is always "GENERATED_KEY". The PostgreSQL issue is that it requires exact case in column name passed to Connection.prepareStatement("insert into IdentityGenerationType (someData) values('gktest')", new String[]

          {"orderid[case matters here]"}

          );

          Looks like the solution is to pass identity column name thru DBDictionary case conversion and to get generated column value by using column index. MySQL, PostgreSQL and DB2 accept this. I will see what about MS SQL Server when I manage to install it. The MS JDBC 2.0 driver suggests it behaves much like MySQL [1].

          [1] http://msdn.microsoft.com/en-us/library/ms378445.aspx

          Show
          Milosz Tylenda added a comment - Omitting an identity column is not the problem. The MySQL issue is that it does not allow the retrieval of generated column by its actual name - coulmn name is always "GENERATED_KEY". The PostgreSQL issue is that it requires exact case in column name passed to Connection.prepareStatement("insert into IdentityGenerationType (someData) values('gktest')", new String[] {"orderid[case matters here]"} ); Looks like the solution is to pass identity column name thru DBDictionary case conversion and to get generated column value by using column index. MySQL, PostgreSQL and DB2 accept this. I will see what about MS SQL Server when I manage to install it. The MS JDBC 2.0 driver suggests it behaves much like MySQL [1] . [1] http://msdn.microsoft.com/en-us/library/ms378445.aspx
          Milosz Tylenda made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Assignee Fay Wang [ faywang ] Milosz Tylenda [ milosz ]
          Hide
          Milosz Tylenda added a comment -

          More work is needed. The problem is that when a table has an identity (auto-increment) column, we omit that column in INSERT statement and then we retrieve the generated (and omitted) column by name. DB2 accepts this but MySQL and PostgreSQL go on strike. Testing on other databases may reveal more oddities.

          I will look into this.

          For those interested, the JDBC driver for PostgreSQL, available in their CVS, supports the getGeneratedKeys variant OpenJPA would use. There is an unofficial build available [1] which contains that feature.

          [1] http://ejurka.com/pgsql/jars/genkey/

          Show
          Milosz Tylenda added a comment - More work is needed. The problem is that when a table has an identity (auto-increment) column, we omit that column in INSERT statement and then we retrieve the generated (and omitted) column by name. DB2 accepts this but MySQL and PostgreSQL go on strike. Testing on other databases may reveal more oddities. I will look into this. For those interested, the JDBC driver for PostgreSQL, available in their CVS, supports the getGeneratedKeys variant OpenJPA would use. There is an unofficial build available [1] which contains that feature. [1] http://ejurka.com/pgsql/jars/genkey/
          Milosz Tylenda made changes -
          Link This issue is blocked by OPENJPA-847 [ OPENJPA-847 ]
          Hide
          Fay Wang added a comment -

          Fix some indentation problem under r701555.

          Show
          Fay Wang added a comment - Fix some indentation problem under r701555.
          Fay Wang made changes -
          Resolution Fixed [ 1 ]
          Status Open [ 1 ] Resolved [ 5 ]
          Hide
          Fay Wang added a comment -

          Fix is committed into trunk r701537.

          Show
          Fay Wang added a comment - Fix is committed into trunk r701537.
          Fay Wang made changes -
          Attachment openjpa-736.patch [ 12391354 ]
          Hide
          Fay Wang added a comment -

          I just found that for PreparedStatement, we can call Connection.PreparedStatement(sql, int) to pass in autoGeneratedKeys. I will explore more about this approach.

          Show
          Fay Wang added a comment - I just found that for PreparedStatement, we can call Connection.PreparedStatement(sql, int) to pass in autoGeneratedKeys. I will explore more about this approach.
          Hide
          Fay Wang added a comment -

          The jdbc api that I could find for the generatedKey is in the Statement, not in the PreparedStatement:

          Statement.executeUpdate(String sql, int autoGeneratedKeys)
          Statement.getGeneratedKeys( ) to get the ResultSet for the generated keys

          OpenJpa uses PreparedStatement, not the Statement. I don't think we should change from PreparedStatement to Statement just to get the generatedKeys.

          Show
          Fay Wang added a comment - The jdbc api that I could find for the generatedKey is in the Statement, not in the PreparedStatement: Statement.executeUpdate(String sql, int autoGeneratedKeys) Statement.getGeneratedKeys( ) to get the ResultSet for the generated keys OpenJpa uses PreparedStatement, not the Statement. I don't think we should change from PreparedStatement to Statement just to get the generatedKeys.
          Hide
          Michael Dick added a comment -

          +1 to use getGeneratedKeys and expand the support to cover all databases.

          Show
          Michael Dick added a comment - +1 to use getGeneratedKeys and expand the support to cover all databases.
          Hide
          Catalina Wei added a comment -

          An alternative is to use db independent jdbc getGeneratedKeys smethod on Statement interface.

          Show
          Catalina Wei added a comment - An alternative is to use db independent jdbc getGeneratedKeys smethod on Statement interface.
          Fay Wang made changes -
          Summary Combine insert and select SQL together when for generated Id with strategy=GenerationType.IDENTITY Combine insert and select SQL together for generated Id strategy=GenerationType.IDENTITY
          Fay Wang made changes -
          Field Original Value New Value
          Assignee Fay Wang [ faywang ]
          Fay Wang created issue -

            People

            • Assignee:
              Milosz Tylenda
              Reporter:
              Fay Wang
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development