Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Implemented
    • Affects Version/s: None
    • Fix Version/s: 1.6
    • Labels:
      None

      Description

      It would be useful to have an insert method on QueryRunner that returns the id of the new record.

      1. QueryRunner_insert.txt
        5 kB
        Moandji Ezana
      2. AsyncQueryRunner_add_insert_methods.patch
        4 kB
        Moandji Ezana

        Activity

        Hide
        William R. Speirs added a comment -

        Code submitted, r1370883

        Show
        William R. Speirs added a comment - Code submitted, r1370883
        Hide
        Moandji Ezana added a comment -

        AsyncQueryRunner_add_insert_methods.patch adds insert methods to AsyncQueryRunner and unit tests verify that call is delegated to the wrapped QueryRunner.

        Show
        Moandji Ezana added a comment - AsyncQueryRunner_add_insert_methods.patch adds insert methods to AsyncQueryRunner and unit tests verify that call is delegated to the wrapped QueryRunner.
        Hide
        Moandji Ezana added a comment -

        I'm waiting for DBUTILS-88 to be resolved before adding the insert methods to AsyncQueryRunner.

        Show
        Moandji Ezana added a comment - I'm waiting for DBUTILS-88 to be resolved before adding the insert methods to AsyncQueryRunner.
        Hide
        William R. Speirs added a comment -

        Yea, I don't have any good ideas for this API. Lets stick with what you have for now, see how that looks and what folks think of it, then we can add additional methods later. Thanks!

        Show
        William R. Speirs added a comment - Yea, I don't have any good ideas for this API. Lets stick with what you have for now, see how that looks and what folks think of it, then we can add additional methods later. Thanks!
        Hide
        Moandji Ezana added a comment -

        "Looking at this patch I noticed that there are two other possible calls which users might want to create the prepared statement:

        prepareStatement(String sql, int[] columnIndexes)
        prepareStatement(String sql, String[] columnNames)"

        What would you see as the API for this? While I agree that those could be useful, I'm not sure it's really necessary:

        1. Multi-column primary keys might be handled by the JDBC driver, if all columns are defined as being part of the primary key. This is pure speculation on my part.

        2. Multi-column primary keys are fairly rare.

        3. Whatever the API we come up with has to be replicated in the AsyncQueryRunner, so it's a lot of extra methods.

        "Otherwise, the patch is missing (as noted above) JavaDocs and the async implementation, but looks sound. If you can update your patch with these additional things, I'll look to commit it. Thanks!"

        Cool. I'll try to make time for it this week.

        Show
        Moandji Ezana added a comment - "Looking at this patch I noticed that there are two other possible calls which users might want to create the prepared statement: prepareStatement(String sql, int[] columnIndexes) prepareStatement(String sql, String[] columnNames)" What would you see as the API for this? While I agree that those could be useful, I'm not sure it's really necessary: 1. Multi-column primary keys might be handled by the JDBC driver, if all columns are defined as being part of the primary key. This is pure speculation on my part. 2. Multi-column primary keys are fairly rare. 3. Whatever the API we come up with has to be replicated in the AsyncQueryRunner, so it's a lot of extra methods. "Otherwise, the patch is missing (as noted above) JavaDocs and the async implementation, but looks sound. If you can update your patch with these additional things, I'll look to commit it. Thanks!" Cool. I'll try to make time for it this week.
        Hide
        William R. Speirs added a comment -

        Looking at this patch I noticed that there are two other possible calls which users might want to create the prepared statement:

        prepareStatement(String sql, int[] columnIndexes)
        prepareStatement(String sql, String[] columnNames)

        This would be in the case that the insert call auto-generates more than one column of keys (a good example of doing that is escaping me now, but ...). With the call in the current patch prepareStatement(String sql, int autoGeneratedKeys) JDBC will, "determine the columns which best represent the auto-generated keys." [1]

        I don't have much experience with this, but do we think this would bite people? It's always hard to tell when it's JDBC driver implementation specific. Thoughts?

        Otherwise, the patch is missing (as noted above) JavaDocs and the async implementation, but looks sound. If you can update your patch with these additional things, I'll look to commit it. Thanks!

        [1] http://docs.oracle.com/javase/6/docs/api/java/sql/Statement.html#getGeneratedKeys()

        Show
        William R. Speirs added a comment - Looking at this patch I noticed that there are two other possible calls which users might want to create the prepared statement: prepareStatement(String sql, int[] columnIndexes) prepareStatement(String sql, String[] columnNames) This would be in the case that the insert call auto-generates more than one column of keys (a good example of doing that is escaping me now, but ...). With the call in the current patch prepareStatement(String sql, int autoGeneratedKeys) JDBC will, "determine the columns which best represent the auto-generated keys." [1] I don't have much experience with this, but do we think this would bite people? It's always hard to tell when it's JDBC driver implementation specific. Thoughts? Otherwise, the patch is missing (as noted above) JavaDocs and the async implementation, but looks sound. If you can update your patch with these additional things, I'll look to commit it. Thanks! [1] http://docs.oracle.com/javase/6/docs/api/java/sql/Statement.html#getGeneratedKeys( )
        Hide
        Moandji Ezana added a comment -

        @Sebb: I agree, but I wanted to get some initial feedback on whether the functionality is desired, implemented in a sane way, etc.

        As soon as that is agreed, I'll fill in the documentation, fix any code style mistakes and also add this functionality to the AsyncQueryRunner and (if possible) batch methods.

        Show
        Moandji Ezana added a comment - @Sebb: I agree, but I wanted to get some initial feedback on whether the functionality is desired, implemented in a sane way, etc. As soon as that is agreed, I'll fill in the documentation, fix any code style mistakes and also add this functionality to the AsyncQueryRunner and (if possible) batch methods.
        Hide
        Sebb added a comment -

        Javadoc needs to be added for the new public methods.

        Show
        Sebb added a comment - Javadoc needs to be added for the new public methods.
        Hide
        Simone Tripodi added a comment -

        The patch is IMHO ready to be applied - maybe needs some works on adding Javadoc and corner cases tests - but it is good.

        I assign the issue to Bill who's the current mastermind behind DbUtils

        Show
        Simone Tripodi added a comment - The patch is IMHO ready to be applied - maybe needs some works on adding Javadoc and corner cases tests - but it is good. I assign the issue to Bill who's the current mastermind behind DbUtils
        Hide
        Moandji Ezana added a comment - - edited

        QueryRunner_insert.txt is a patch that adds the insert methods and a very basic unit test, just to get the ball rolling.

        Show
        Moandji Ezana added a comment - - edited QueryRunner_insert.txt is a patch that adds the insert methods and a very basic unit test, just to get the ball rolling.

          People

          • Assignee:
            William R. Speirs
            Reporter:
            Moandji Ezana
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development