Add support for returning clause, when running plain SQL

[lukaseder]

When using plain SQL, users are not able to return generated IDs in examples like these:

ctx.execute("insert into T(A, B) values (1, 2)");
ctx.query("insert into T(A, B) values (1, 2)").execute();

While some databases support a native RETURNING clause:

ctx.fetch("insert into T(A, B) values (1, 2) returning ID");

Others need JDBC's Statement.getGeneratedKeys() for this. jOOQ should support an additional returning() clause, which transforms any Query into a ResultQuery:

interface Query {
    ResultQuery<Record> returning(Field<?>... fields);
}

---

See also:

• H2 Support for returning primary key from inserts when using DSL #10396 H2 Support for returning primary key from inserts when using DSL
• Use DataType.identity() information in the absence of Table.getIdentity() in INSERT .. RETURNING emulation #10400 Use DataType.identity() information in the absence of Table.getIdentity() in INSERT .. RETURNING emulation

[shubham-vl]

When this issue is going to be resolved ??

[lukaseder]

Hi @shubham-vl. Thank you very much for your message. Implementing this feature is, very unfortunately, more complex than it may initially seem, especially in the context of the 21 RDBMS that jOOQ currently supports.

However, there's always a workaround. If you're using PostgreSQL, for instance, you can put the RETURNING clause directly in your plain SQL string.

I hope this helps

[shubham-vl]

For mysql is there something like this or not ??

[lukaseder]

No, not right now

[shubham-vl]

Ok. Thanks Regards Shubham Jaju

…

On Mon, Apr 24, 2017 at 4:10 PM, Lukas Eder ***@***.***> wrote: No, not right now — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2943 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AS200-hHF0_muU-3kpPkq4yaiN2cgHxjks5rzHwfgaJpZM4BZPWm> .

[grzegorzbor]

Hi, I'm now in the process of migrating our persistence layer to JOOQ, and I'm stuck on the problem described above.
We're using MariaDB. We have a universal method which takes plain SQL plus optional params, and it returns generated ID.
Till now we used the JDBC API:
connection.prepareStatement(query, Statement.RETURN_GENERATED_KEYS);
//...bind and execute...
statement.getGeneratedKeys()
But I can't do it in JOOQ.

The only way of doing it I found, is by doing it all manually inside ConnectionCallable, but I have to manually bind params, loose logging functionality etc, so basically I lose all JOOQ's goodies.

The way I think it should work would be something like:
int generatedId = ctx.insertResultQuery(sql, params).fetchInto(Integer.class)
The "insertResultQuery" would make sure the statement is created with Statement.RETURN_GENERATED_KEYS and the method returns statement.getGeneratedKeys().

In the meantime, I tried using ExecuteListener to call statement.getGeneratedKeys() after the insert has been executed, but I'm getting exception because the statement has been created without RETURN_GENERATED_KEYS flag.
So alternatively, as a workaround, it would be nice to have ability to influence the way the statement is created. This can be done by adding a new Setting like "createStatmentWithReturnGeneratedKeys"
And then I can do:

configuration
.set(new Settings().withCreateStatmentWithReturnGeneratedKeys(true))
.set(new ExecutorListenerRetrievingGeneratorKeys())

[lukaseder]

@grzegorzbor Your suggestion is well intended, but unfortunately, it is incomplete. It may work for you immediately, but not for many others working with other databases, where an "insert result query" returns results not through Statement.getGeneratedKeys() but Statement.getMoreResults(), and by consequence, Statement.RETURN_GENERATED_KEYS shouldn't be used in the first place.

I wouldn't get too invested in this rather funky JDBC feature. Configuring / tweaking things at this level will be way too complicated to document / maintain / use.

If there's any solution at all, it must be completely transparent, work on all databases, and not be surprising. This is simply not a priority to solve right now, I'm afraid.

What keeps you from using the code generator, by the way?

[grzegorzbor]

The reason for not using code generator is that we are migrating the old project, where there are thousands of plain-text queries. Migrating them would take ages. So what we do, is that we want to change the database access layer to use JOOQ internally, but still have this layer providing a method which takes any insert query (as an opaque String), and returns generated ID. The old implementation used the Statement.getGeneratedKeys() for this purpose.

I understand it may be too complex to make it work for all the databases; on the other hand, completely blocking a feature which works nicely in a partical database (mysql), is too restrictive. As i mentioned, I found a workaround by using ExecutorListener to plug into the lifecycle and call the getGeneratedKeys() after query was executed. That's totally fine with me, the only thing I need is also a way to override the place in the code which calls connection.createStatement and connection.prepareStatement, and pass Statement.RETURN_GENERATED_KEYS flag to it - without that, it won't work.
The simplest solution I think is to add additional Setting like "createStatementWithGeneratedKeyFlag" - nobody is forced to use it. The other option is to have a new interface "StatementCreator" - which gives you fill control on how the statement is created. Something similar to Spring's PreparedStatementCreator, though more general. Then I can register my own implementation on the Configuration object. This way it is fully database-engine independent.

There are several methods to create a Statement, so the interface should probably have all of them. Something like below:

interface StatementCreator {
  Statement createStatement(Connection conn) throws SQLException;
  Statement createStatement(Connection conn, int resultSetType, int resultSetConcurrency)
  PreparedStatement prepareStatement(Connection conn, String sql) throws SQLException;
  PreparedStatement prepareStatement(Connection conn, String sql, int resultSetType, int resultSetConcurrency) throws SQLException;
  PreparedStatement prepareStatement(Connection conn, String sql, int columnIndexes[]) throws SQLException;
  PreparedStatement prepareStatement(Connection conn, String sql, String columnNames[]) throws SQLException;
}

This way is quite non-intrusive, because most of the users will just use a default implementation without even being aware of other options.

[lukaseder]

Migrating them would take ages.

Sure, but I'm assuming you will migrate eventually, so why not migrate these particular queries already? I'm quite positive that this will be done quicker than an appropriate implementation for this feature here.

completely blocking a feature which works nicely in a partical database (mysql), is too restrictive

That's easy for you to say :) You'd be happy with any feature that would get you this block out of the way. We'll have to maintain it for the next 10 years.

As i mentioned, I found a workaround by using ExecutorListener to plug into the lifecycle and call the getGeneratedKeys() after query was executed. That's totally fine with me, the only thing I need is also a way to override the place in the code which calls connection.createStatement and connection.prepareStatement, and pass Statement.RETURN_GENERATED_KEYS flag to it - without that, it won't work.

If you're open to that level of patching, then you could proxy the JDBC API and replace jOOQ's ordinary prepareStatement() calls with calls that actually use the flag, whenever you need that. jOOQ offers some utilities called DefaultConnection and DefaultStatement to help you with this proxying task.

Specifically, you could write:

class MyConnection extends DefaultConnection {
    // ...
    @Override
    public PreparedStatement prepareStatement(String sql) throws SQLException {
        if (someFlagActivatingGeneratedKeys)
            return super.prepareStatement(sql, Statement.RETURN_GENERATED_KEYS);
        else
            return super.prepareStatement(sql);
    }
}

This actually works just like your StatementCreator idea.

I'd really prefer this approach over actual new API, because I'm not convinced that this kind of API will be very useful to a lot of users compared to the cost of maintaining the API.

[grzegorzbor]

Ok thanks for the hint, this works for me. I had to to wrap a couple of things (ConnectionProvider, Connection itself, and Statement) but it's working fine now.

[lukaseder]

Thanks for the feedback. I have created a separate feature request for your original idea. Perhaps, there is a way to enhance the ExecuteListener SPI to allow implementations to override the statement creation: #7103

[grzegorzbor]

Hi, is there any plan for progressing with that issue?

To recap, my scenario is following:
We have a big application, with hundreds of tables, running on MySQL/MariaDB. Using the generated static tables is not an option, for several reasons. My aim is to use jooq as an engine for running SQL queries across all the places in the code, gradually replacing the plain JDBC. But inability to retrieve generated IDs for inserts is a real issue. We implemented a workaround, with custom ExecutionListeners, providers, callback etc. But it's really awkward and annoying. It should be much easier. JDBC provides a standard getGeneratedKeys() method for it, so there should be a way to use it from jooq with those databases which support it.

I think it should work liek this:
Record generated = dsl.query("insert into foo values 1").returningGenKeys().fetchOne();

The new "returningGenKeys()" method transfers the Query into ResultQuery, but the actual ResultSet is going to be retrieved by calling underlying getGeneratedKeys(). The returningGenKeys method would also make sure the underlying Statement is created with the RETURN_GENERATED_KEYS flag.

[lukaseder]

Thanks for following up on this, @grzegorzbor.

Ultimately, the problem will boil down to finding a way to communicate the identity column to jOOQ, when running this kind of statement - and the fact that there even is such a thing. With plain SQL as you did it, this will be much more difficult to get right than with the meta model. If you use the code generator, that meta model is generated for you. You could write it programmatically yourself, and use internal API - or hopefully soon, a better public API for meta model construction (see #7444).

I understand your argument comparing JDBC's getGeneratedKeys() method to this, but trust me, it is not this simple. That part of JDBC is not implemented very well by most drivers:

• Some drivers (or DBs) support only single row generated keys
• Some drivers (or DBs) support the feature only for INSERT statements, not UPDATE or MERGE statements
• Some drivers (or DBs) support this feature only for "real" identity columns, not trigger generated ones
• Some drivers (or DBs) support this feature only for the identity column itself, not for all the other columns of the inserted (or updated) row.

When using the meta model, and type safe querying, most of the above issues are being worked around by jOOQ. When using plain SQL, it will be a mess again, just like in JDBC. You may have your feature, but others will find that just as annoying.

This is why what you perceive as a really simple addition is in fact not that simple.

Using the generated static tables is not an option, for several reasons

Maybe, would you like to elaborate on this? Because using the DSL with generated code really does solve your problems most thoroughly, so I'd like to understand why that's not an option.

Of course, another workaround is this:

public static Record generatedKey(String sql) {
    return dsl.connectionResult(c -> {
        try (PreparedStatement ps = c.prepareStatement(sql, RETURN_GENERATED_KEYS)) {
            ps.execute();
            try (ResultSet rs = ps.getGeneratedKeys()) {
                return dsl.fetchOne(rs);
            }
        } 
    });
}

That doesn't look too bad, does it?

[grzegorzbor]

The workaround is helpful, though using it, i'm losing some of nice Jooq features: binding of the statement parameters, and logging of the executed statement (with those params). Though it's still an option to consider.

Generally, I understand your point and your constraints as a framework author. But I don't agree with two points.

1. I don't see a need for enforcing consistent behavior of given feature across all databases and all tricky scenarios. Most of the projects I ever seen, just use one type of database. And definitely most of the projects do not use generated keys with UDPATE or MERGE statements (actually i can barely recollect I've ever seen it being used). 99% of cases when you need a generated key, is for simple insert statement. In other words, if your're using a database which does not work nicely with generated keys, or you have a very special case, you are not going to be interested in using this feature anyway. But for a typical scenario when you just want to run a simple inserts and that's all that you need, you are happy if this feature is not masked by your framework.

2. I believe, using generated classes with jooq should be an optional, not mandatory. Jooq is a nice sql-execution layer masking awkward low-level JDBC API. Generated classes and type-safe queries are nice, but they should be optional, because they've got downsides as well:
   a) Plain text SQL queries are lingua franca, you may just copy them from your code and send to a DBA, to a data analyst, or just copy it yourself to your sql tool to run in against your database, to check the results it returns. I have quite commonly queries that join several tables with complex join criteria. I write them as plain SQL as that is easier to write, analyze and test. Then I translate them to jooq's DSL. After some time, when I need to quickly debug a data issue, I copy them back from the DSL form in the code, manually convert to plain sql, and run and experiment with it using SQL tool. In fact at some point I stopped using DSL queries in the code, and switched back to using plain SQLs. This saves my time and is directly shareable with people who understand sql but are not java devs.
   b) In our case, the projects is big, and uses an integration-style database with hundreds of tables with tons of columns, used be different teams. One team is only interested in a subset of tables, and subset of their columns. The columns may come and go, they are changing quite often. Also, there a different environments, with different versions of that database. You don't really want to generate classes for all of them, because the cost of maintaining them would be too high.

Point b) is debatable and is definitely not a recommended software architecture, but world is not perfect, and I'm sure it is not an uncommon case. But even in a perfect world, in a greenfield app, point a) would make me consider using plain SQLs instead of DSL with generated classes.

[lukaseder]

I don't see a need for enforcing consistent behavior of given feature across all databases and all tricky scenarios.

You don't because you only use MySQL :)

Most of the projects I ever seen, just use one type of database

I disagree here.

And definitely most of the projects do not use generated keys with UDPATE or MERGE statements

Most of the projects using jOOQ use the code generator... So, if we draw a Venn diagram...

But for a typical scenario when you just want to run a simple inserts and that's all that you need, you are happy if this feature is not masked by your framework.

I never disagreed. I just told you about a couple of reasons why things aren't as simple as you thought. This doesn't mean I don't recognise the utility of supporting this kind of feature.

I believe, using generated classes with jooq should be an optional, not mandatory

It totally is. But some features are really just much simple to implement if meta data is available.

a) Plain text SQL queries are lingua franca, you may just copy them from your code and send to a DBA, to a data analyst

Sure, but jOOQ's main value proposition is the DSL. I mean, if you're just here for a bit of JDBC pain easing, then JDBI or Spring JdbcTemplate could've been a choice as well. You did choose jOOQ, though, because it can do much more, right?

If you do want to write complex, re-usable SQL with jOOQ, one great option is to use views. An interesting application design model that works very well with jOOQ is this one:
https://www.salvis.com/blog/2018/07/18/the-pink-database-paradigm-pinkdb/

You don't really want to generate classes for all of them, because the cost of maintaining them would be too high.

I don't fully understand this argument :) I mean, if columns "come and go", it's a great thing to know that your code breaks at compile time, right? If columns "go" and your SQL code still references it, you won't notice the bug until you run the query. So, jOOQ definitely adds value here...

In any case. Again, rest assured, it would be great indeed for plain SQL queries to be able to fetch generated keys also from database products that do not support a clause like PostgreSQL's INSERT .. RETURNING. But again, implementing this feature really isn't that simple as you made it look. Otherwise, we would definitely already have it!

[shumy-tools]

Searching for the perfect solution... In the meanwhile we have none.
If the problem is to identify the identity field, I would be at least pleased with a minor solution such as:

DataType<Long> type = DefaultDataType.getDataType(SQLDialect.DEFAULT, java.lang.Long.class, true)
field(name("id"), type)
// or 
...returningId(field(...)).fetch()

[lukaseder]

Thank you very much for your suggestion, @shumy-tools. It should be possible to use SQLDataType.BIGINT.identity(true), but it currently isn't.

I'll look into whether this can be done for jOOQ 3.14

[lukaseder]

A workaround has been implemented in #10400, which may not work for plain SQL queries like in this issue's description, but for cases like this:

ctx.insertInto(table("t"))
   .columns(field("col", INTEGER))
   .values(1)
   .returning(field("id", INTEGER.identity(true)))
   .fetch();