Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Core
    • Labels:
      None

      Description

      public int executeUpdate(String sql,int autoGeneratedKeys)
      public boolean execute(String sql,int autoGeneratedKeys)
      public ResultSet getGeneratedKeys()

      We should provide support for jdbc auto generated keys retrieval. This is only available in Java 1.4. So, we would have to throw an UnsupportedFeatureException if 1.3 or earlier was being used.

      Brandon

      1. 2008-03-31-inwork.patch
        13 kB
        Brian Diekelman
      2. ibatis-2.3.4.726-src.diff
        11 kB
        Derek Hulley
      3. ibatis-jdbc3-patch.txt
        15 kB
        Zach Visagie

        Activity

        Hide
        Matt Raible added a comment -

        I'd love to get getGeneratedKeys() supported so I don't have to hard-code database-specific code to return primary keys in my SQL mapping files.

        Show
        Matt Raible added a comment - I'd love to get getGeneratedKeys() supported so I don't have to hard-code database-specific code to return primary keys in my SQL mapping files.
        Hide
        Niels Beekman added a comment -

        In theory, this could be implemented since 1.4 is default now. However, not all DBMS drivers support this, for example the PostgreSQL driver doesn't. I've been using it succesfully with the JTDS MSSQL driver. To overcome hardcoding database-specific code, we usually put all auto-increment columns in a properties file, which we fill for each supported DBMS.

        Show
        Niels Beekman added a comment - In theory, this could be implemented since 1.4 is default now. However, not all DBMS drivers support this, for example the PostgreSQL driver doesn't. I've been using it succesfully with the JTDS MSSQL driver. To overcome hardcoding database-specific code, we usually put all auto-increment columns in a properties file, which we fill for each supported DBMS.
        Hide
        Clinton Begin added a comment -

        Niels has an excellent solution. I never thought of that (oh the shame!). $

        {SELECT_KEY}

        or something like that.

        Anyway...

        I think it is time we did this, especially now that (as Niels said) 1.4 is required (come on people, Java 6 is out!).

        It shouldn't be very hard at all. I'll have a look in the next week. What the heck, I'm on vacation.

        Cheers,
        Clinton

        Show
        Clinton Begin added a comment - Niels has an excellent solution. I never thought of that (oh the shame!). $ {SELECT_KEY} or something like that. Anyway... I think it is time we did this, especially now that (as Niels said) 1.4 is required (come on people, Java 6 is out!). It shouldn't be very hard at all. I'll have a look in the next week. What the heck, I'm on vacation. Cheers, Clinton
        Hide
        Matt Raible added a comment -

        Niels - do you have a more descriptive example of how you do this? A blog post or wiki-page writeup would be great.

        Show
        Matt Raible added a comment - Niels - do you have a more descriptive example of how you do this? A blog post or wiki-page writeup would be great.
        Hide
        Claus Ibsen added a comment -

        As a early adaptor of iBatis in 2003 I really loved this framework.

        Now I have the chance again to work with it and I'm a bit surprised it hasn't evolved as much as expected.
        Well the foundation from the start was really great, so that could be it

        So this get my vote as 1.4 is now WebSphere 5.x standard, and to get these keys is something we all have to do.

        Show
        Claus Ibsen added a comment - As a early adaptor of iBatis in 2003 I really loved this framework. Now I have the chance again to work with it and I'm a bit surprised it hasn't evolved as much as expected. Well the foundation from the start was really great, so that could be it So this get my vote as 1.4 is now WebSphere 5.x standard, and to get these keys is something we all have to do.
        Hide
        Brandon Goodin added a comment -

        Is there any reason why we can't expand our existing selectKey syntax and add an attribute called generatedKeys="true/false/yes/no"? That way we retain the mapping ability and ditch the body. <selectKey resultClass="int" keyProperty="id" generatedKeys="true"/>. I know the executeUpdate/execute has the potential to return multiple results. But, is that a scenario that will happen when using iBATIS?

        Show
        Brandon Goodin added a comment - Is there any reason why we can't expand our existing selectKey syntax and add an attribute called generatedKeys="true/false/yes/no"? That way we retain the mapping ability and ditch the body. <selectKey resultClass="int" keyProperty="id" generatedKeys="true"/>. I know the executeUpdate/execute has the potential to return multiple results. But, is that a scenario that will happen when using iBATIS?
        Hide
        Tony Jewell added a comment -

        This is causing impact in my project now and as we are weighing up staying with ibatis or moving to Spring Jdbc. Lack of generated key support could be significant.

        The using of the SELECT @@identity specific for sybase is annoying and just doesn't feel right.

        So - yes it would be cool. You have the return structure from the execute - the returned object could be a collection - just some syntactic sugar in the sqlmap <selectkey as Brandon suggests to enable it, optionally providing the names of the auto-generated keys to return.

        Show
        Tony Jewell added a comment - This is causing impact in my project now and as we are weighing up staying with ibatis or moving to Spring Jdbc. Lack of generated key support could be significant. The using of the SELECT @@identity specific for sybase is annoying and just doesn't feel right. So - yes it would be cool. You have the return structure from the execute - the returned object could be a collection - just some syntactic sugar in the sqlmap <selectkey as Brandon suggests to enable it, optionally providing the names of the auto-generated keys to return.
        Hide
        Matt Raible added a comment -

        Any update on this issue?

        Show
        Matt Raible added a comment - Any update on this issue?
        Hide
        Clinton Begin added a comment -

        It's only been 2 years man, give us some time!

        LOL. It should not be a problem now that we've stopped supporting JDK 1.3. I think it's fairly simple, just a matter of finding the time.

        Clinton

        Show
        Clinton Begin added a comment - It's only been 2 years man, give us some time! LOL. It should not be a problem now that we've stopped supporting JDK 1.3. I think it's fairly simple, just a matter of finding the time. Clinton
        Hide
        Venkatt Guhesan added a comment -

        I would like to see this feature implemented. And plus now that we are moving to Spring's DAO.. Is there something built within Spring that makes this feasable?

        Show
        Venkatt Guhesan added a comment - I would like to see this feature implemented. And plus now that we are moving to Spring's DAO.. Is there something built within Spring that makes this feasable?
        Hide
        Brian Diekelman added a comment - - edited

        Alright, I guess somebody has to bite the bullet and create a patch for this... how about this:

        Single Generated Key:
        Using generated in the 'type' attribute since the other options (pre/post) are implied by generated (which has to be post)
        <insert id="insertUser" parameterClass="user">
        insert into user_profiles(first_name_tx)values(#firstName#)

        <selectKey resultClass="int" type="generated" keyProperty="id"/>
        </insert>

        Multiple Generated Keys:
        The case of a trigger (or several sql statements if the driver supports it)
        with multiple generated keys, this would apply them in order (if we got the int[]
        back with two entries, id=[0], otherId=[1])

        <insert id="insertUser" parameterClass="user">
        insert into user_profiles(first_name_tx)values(#firstName#)

        <selectKey resultClass="int" type="generated" keyProperty="id"/>
        <selectKey resultClass="int" type="generated" keyProperty="otherId"/>
        </insert>

        Code changes
        --------------------
        New Classes:

        • create a com.ibatis.sqlmap.engine.mapping.statement.GeneratedKeyStatement

        Changes to Existing Files:

        • com.ibatis.sqlmap.engine.mapping.statement.InsertStatement
        • added a GeneratedKeyStatment[] field + getters/setters
        • com.ibatis.engine.builder.xml.SqlStatementParser.parseSelectKey()
        • add generated key logic, add to insertStatement
        • com.ibatis.engine.impl.SqlMapExecutorDelegate
        • line 424, inside insert(SessionScope, String, Object)
        • add logic to check for selectKeyStatement/generatedKeyStatement
        • add 'generated' to 'selectKey' in sql-map-2.dtd

        Outstanding questions to think through:

        • Throw an exception if we have a selectKey with type 'generated' and no keys are generated? (optional setting?)
        • Allow a pre/post selectKey and a generated selectKey in the same statement?

        If I get some halfway decent feedback I might be able to get a patch ready over the weekend. If nothing else, hopefully this will leave a blueprint of what to change for the next person if I fail miserably.

        Show
        Brian Diekelman added a comment - - edited Alright, I guess somebody has to bite the bullet and create a patch for this... how about this: Single Generated Key: Using generated in the 'type' attribute since the other options (pre/post) are implied by generated (which has to be post) <insert id="insertUser" parameterClass="user"> insert into user_profiles(first_name_tx)values(#firstName#) <selectKey resultClass="int" type="generated" keyProperty="id"/> </insert> Multiple Generated Keys: The case of a trigger (or several sql statements if the driver supports it) with multiple generated keys, this would apply them in order (if we got the int[] back with two entries, id= [0] , otherId= [1] ) <insert id="insertUser" parameterClass="user"> insert into user_profiles(first_name_tx)values(#firstName#) <selectKey resultClass="int" type="generated" keyProperty="id"/> <selectKey resultClass="int" type="generated" keyProperty="otherId"/> </insert> Code changes -------------------- New Classes: create a com.ibatis.sqlmap.engine.mapping.statement.GeneratedKeyStatement Changes to Existing Files: com.ibatis.sqlmap.engine.mapping.statement.InsertStatement added a GeneratedKeyStatment[] field + getters/setters com.ibatis.engine.builder.xml.SqlStatementParser.parseSelectKey() add generated key logic, add to insertStatement com.ibatis.engine.impl.SqlMapExecutorDelegate line 424, inside insert(SessionScope, String, Object) add logic to check for selectKeyStatement/generatedKeyStatement add 'generated' to 'selectKey' in sql-map-2.dtd Outstanding questions to think through: Throw an exception if we have a selectKey with type 'generated' and no keys are generated? (optional setting?) Allow a pre/post selectKey and a generated selectKey in the same statement? If I get some halfway decent feedback I might be able to get a patch ready over the weekend. If nothing else, hopefully this will leave a blueprint of what to change for the next person if I fail miserably.
        Hide
        Venkatt Guhesan added a comment -

        Hey Brian,

        Your game plan sounds good. I'm looking forward to this fix. This will make the configuration easier and compatible with the features available in JDK 1.4. I'm a MySQL user but I occasionally use Postgres as well...

        Since this feature is not available in Postgresql and pre-JDK 1.4, my suggestion would be that if someone tried using this format against Postgresql or a database that does not have this feature or a pre-1.4 JDK, then you can query the

        boolean java.sql.DatabaseMetaData.supportsGetGeneratedKeys() and if it returns a false then you can throw an "UnsupportedFeatureException"
        or if the JVM < 1.4 you can throw the same "UnsupportedFeatureException".

        This can potentially eliminate some configuration errors.

        Show
        Venkatt Guhesan added a comment - Hey Brian, Your game plan sounds good. I'm looking forward to this fix. This will make the configuration easier and compatible with the features available in JDK 1.4. I'm a MySQL user but I occasionally use Postgres as well... Since this feature is not available in Postgresql and pre-JDK 1.4, my suggestion would be that if someone tried using this format against Postgresql or a database that does not have this feature or a pre-1.4 JDK, then you can query the boolean java.sql.DatabaseMetaData.supportsGetGeneratedKeys() and if it returns a false then you can throw an "UnsupportedFeatureException" or if the JVM < 1.4 you can throw the same "UnsupportedFeatureException". This can potentially eliminate some configuration errors.
        Hide
        Brian Diekelman added a comment -

        I've been working on the java_release_2.3.0-677 tag. The trunk appeared to have a lot of changes to it and I wanted a clean patch that could be applied to the latest release.

        The biggest problem thus far is how deep the actual PreparedStatement.execute() is called:

        SqlMapSessionImpl.insert()
        SqlMapExecutorDelegate.insert()
        GeneralStatement.executeUpdate()
        GeneralStatement.sqlExecuteUpdate()
        SqlExecutor.executeUpdate()

        That last call prepares a PreparedStatement, calls execute(), gets the update count, then closes it. Great for efficiency, bad if we need to get generated keys from the PreparedStatement before it's closed.

        My plan now is to add: afterExecute(RequestScope request, PreparedStatement) to InsertStatement to be called by SqlExecutor.executeUpdate() if the MappedStatement in the RequestScope is an instance of InsertStatement.

        The majority of my time so far has been finding the most non-intrusive way to add this without any unintended consequences affecting the otherwise stable codebase. Hopefully I'll have a couple more hours to work on this over the next few days, if not it'll be my project for next weekend.

        Show
        Brian Diekelman added a comment - I've been working on the java_release_2.3.0-677 tag. The trunk appeared to have a lot of changes to it and I wanted a clean patch that could be applied to the latest release. The biggest problem thus far is how deep the actual PreparedStatement.execute() is called: SqlMapSessionImpl.insert() SqlMapExecutorDelegate.insert() GeneralStatement.executeUpdate() GeneralStatement.sqlExecuteUpdate() SqlExecutor.executeUpdate() That last call prepares a PreparedStatement, calls execute(), gets the update count, then closes it. Great for efficiency, bad if we need to get generated keys from the PreparedStatement before it's closed. My plan now is to add: afterExecute(RequestScope request, PreparedStatement) to InsertStatement to be called by SqlExecutor.executeUpdate() if the MappedStatement in the RequestScope is an instance of InsertStatement. The majority of my time so far has been finding the most non-intrusive way to add this without any unintended consequences affecting the otherwise stable codebase. Hopefully I'll have a couple more hours to work on this over the next few days, if not it'll be my project for next weekend.
        Hide
        Brian Diekelman added a comment -

        My free time has disappeared the last few weeks due to 'real' work and will probably remain that way for the next few weeks.

        I'll try to revisit this later, but for now I just don't have a lot of time to work on it.

        I've gotten quite a few emails about my progress on this. I've attached a patch with a bunch of 'FIXME (Brian)' comments that should let someone else pick up where I left off (hopefully someone more familiar with iBatis internals).

        Show
        Brian Diekelman added a comment - My free time has disappeared the last few weeks due to 'real' work and will probably remain that way for the next few weeks. I'll try to revisit this later, but for now I just don't have a lot of time to work on it. I've gotten quite a few emails about my progress on this. I've attached a patch with a bunch of 'FIXME (Brian)' comments that should let someone else pick up where I left off (hopefully someone more familiar with iBatis internals).
        Hide
        Zach Visagie added a comment - - edited

        I have included the following patch: ibatis-jdbc3-patch.txt

        It is for adding jdbc3 support and was derived from ibatis 2.3.2. It's a real hack, since I use a statically accessed ThreadLocal to pass the generated key across layers of interfaces and is only meant for the desperate (like me). This is not intended for inclusion in ibatis, but merely to get jdbc3 generated keys working quickly and works only with jdbc3 prepareStatement( String sql, String[] columns ) setting a single column in that array.

        No config dtd's have been modified and it uses selectKey to pass in the column name with a prefix:

        <selectKey keyProperty="id" resultClass="java.lang.Long">
        KEY_COLUMN: mytable_id
        </selectKey>

        The type attribute is ignored. So pre or post becomes irrelevant in this scenario.
        It was the least intrusive way I could just get it working, and in this way will still work with abator. All standard features should still work and it passed all the unit tests.

        Show
        Zach Visagie added a comment - - edited I have included the following patch: ibatis-jdbc3-patch.txt It is for adding jdbc3 support and was derived from ibatis 2.3.2. It's a real hack, since I use a statically accessed ThreadLocal to pass the generated key across layers of interfaces and is only meant for the desperate (like me). This is not intended for inclusion in ibatis, but merely to get jdbc3 generated keys working quickly and works only with jdbc3 prepareStatement( String sql, String[] columns ) setting a single column in that array. No config dtd's have been modified and it uses selectKey to pass in the column name with a prefix: <selectKey keyProperty="id" resultClass="java.lang.Long"> KEY_COLUMN: mytable_id </selectKey> The type attribute is ignored. So pre or post becomes irrelevant in this scenario. It was the least intrusive way I could just get it working, and in this way will still work with abator. All standard features should still work and it passed all the unit tests.
        Hide
        Zach Visagie added a comment -


        I reattach the patch since I forgot to grant ASF licence. It is kindly donated by iPay's BizSwitch Development Team.

        Show
        Zach Visagie added a comment - I reattach the patch since I forgot to grant ASF licence. It is kindly donated by iPay's BizSwitch Development Team.
        Hide
        Derek Hulley added a comment -

        Hi, All

        I have added a patch ibatis-2.3.4.726-src.diff, which is based on the Zach Visagie's contribution, but with a few differences and a notable bug fix.

        The "selectKey" statement is run in it's proper pre/post location. Clearly for generated keys, the statement should be afterwards.
        <selectKey resultClass="long" keyProperty="id" type="post">
        KEY_COLUMN:id
        </selectKey>
        The KEY_COLUMN varies from DB to DB, for instance, with SQLServer, you would need
        KEY_COLUMN:id
        but for MySQL, you use:
        KEY_COLUMN:GENERATED_KEY

        I have also added in type conversion from the generated key type to the target object setter type. If SQLServer returns a BigDecimal, it will still be acceptable to have a different ID type:
        setId(Long id)

        { ...}

        method on the target object. The behaviour is thus the same as as if a proper post-select were run, except that one isn't.

        This is far from good code and a low-level sweep by iBatis developers would be required. It works, though.

        Show
        Derek Hulley added a comment - Hi, All I have added a patch ibatis-2.3.4.726-src.diff, which is based on the Zach Visagie's contribution, but with a few differences and a notable bug fix. The "selectKey" statement is run in it's proper pre/post location. Clearly for generated keys, the statement should be afterwards. <selectKey resultClass="long" keyProperty="id" type="post"> KEY_COLUMN:id </selectKey> The KEY_COLUMN varies from DB to DB, for instance, with SQLServer, you would need KEY_COLUMN:id but for MySQL, you use: KEY_COLUMN:GENERATED_KEY I have also added in type conversion from the generated key type to the target object setter type. If SQLServer returns a BigDecimal, it will still be acceptable to have a different ID type: setId(Long id) { ...} method on the target object. The behaviour is thus the same as as if a proper post-select were run, except that one isn't. This is far from good code and a low-level sweep by iBatis developers would be required. It works, though.
        Hide
        Zach Visagie added a comment -

        Thanks Derek, I also discovered the type conversion issue some issues here and there. I forgot to contribute it back to this page though, but I'll have a look sometime. It would be great if some of the ibatis devs had a look whether it might not break some odd/rare scenarios. It's definitely use at own risk!

        I realised how much work it would be to do a proper solution, so I do understand why it's not been done yet. (:

        Show
        Zach Visagie added a comment - Thanks Derek, I also discovered the type conversion issue some issues here and there. I forgot to contribute it back to this page though, but I'll have a look sometime. It would be great if some of the ibatis devs had a look whether it might not break some odd/rare scenarios. It's definitely use at own risk! I realised how much work it would be to do a proper solution, so I do understand why it's not been done yet. (:
        Hide
        Clinton Begin added a comment -

        iBATIS 3 has proper support for generated keys. Unfortunately it was an afterthought for so many JDBC drivers (still is – even for Derby, the "Java DB"!), that it became an afterthought of ours as well.

        Show
        Clinton Begin added a comment - iBATIS 3 has proper support for generated keys. Unfortunately it was an afterthought for so many JDBC drivers (still is – even for Derby, the "Java DB"!), that it became an afterthought of ours as well.

          People

          • Assignee:
            Unassigned
            Reporter:
            Brandon Goodin
          • Votes:
            11 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:

              Development