INSERT .. RETURNING always returns null for dynamically created tables

[Sven Jacobs]

This is a continuation of the discussion about the same issue.

I recently discovered a problem when using insertInto() with returning() and dynamically created tables, when not using jOOQ's code generation feature.

I'm not using the code generation because the underlying database is only known during runtime and might change.

In that case the returned Record, which should contain the generated ID of the newly inserted row, is always null. That's because in AbstractStoreQuery#selectReturning() the check if (into.getIdentity() != null) is always false since the dynamically generated table doesn't have an identity set.

// record is always null

Record record = create.insertInto(tableByName("MY_TABLE"), fieldByName("FIELD1"), fieldByName("FIELD2"))

    .values(values)

    .returning(fieldByName("ID_FIELD"))

    .fetchOne();

Lukas pointed me to CustomTable and also made it possible to overwrite getIdentity() by subtypes but I still don't understand how I can use this to fix my problem.

Ok, I can now create an anonymous instance of CustomTable during runtime that represents the table and also specify an identity, but how do I add fields to my custom table? Because that would be the next issue of selectReturning(). Utils.newRecord() expects the table to return its fields via the fields() method that however is declared final in AbstractTable.

I'm sure I'm just overlooking something here... Thanks your any help!

Sven

[Sven Jacobs]

What I missed is that in AbstractTable#createField() a new instance TableFieldImpl is created and that this instance is added to the table's fields in the constructor of TableFieldImpl itself.

I didn't expect this and to be honest think this is a bit dirty. Why does a TableField add itself to a table? Seems to be a mix-up of responsibility here. Wouldn't it be cleaner if the instance is added to the fields in createField()?

[Sven Jacobs]

Hey, it's me again :)

I'm now trying to create my own (temporary) table based on TableImpl

class TemporaryTable extends TableImpl {

private final String idColumn;

TemporaryTable(final String name,

               final String idColumn) {

super(name);

this.idColumn = idColumn;

}

@Override

public Identity getIdentity() {

return new Identity() {

@Override

public Table getTable() {

return TemporaryTable.this;

}

@Override

public TableField getField() {

// What to return here?!

}

};

}

public <T> void addField(final String name, final DataType<T> dataType) {

createField(name, dataType, this);

}

}

but I don't know what to return at getField()? TableFieldImpl is package-protected so I cannot instantiate it and I'm surely not going to implement all methods of TableField and the ones inherited by Field ;)

[Lukas Eder]

Hi Sven,

Yes, it's dirty and awful. When I wrote that, I hoped that no one will
ever discover it ;-)
Feel free to provide a patch to this issue.

2013/4/5 Sven Jacobs <sj1...@gmail.com>:

> --
> You received this message because you are subscribed to the Google Groups
> "jOOQ User Group" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to jooq-user+...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[Lukas Eder]

Hi Sven,

You're having fun here? :-)

2013/4/5 Sven Jacobs <sj1...@gmail.com>:

> Hey, it's me again :)
>
> I'm now trying to create my own (temporary) table based on TableImpl
>
> class TemporaryTable extends TableImpl {
> private final String idColumn;
>
> TemporaryTable(final String name,
> final String idColumn) {
> super(name);
> this.idColumn = idColumn;
> }
>
> @Override
> public Identity getIdentity() {
> return new Identity() {
> @Override
> public Table getTable() {
> return TemporaryTable.this;
> }
>
> @Override
> public TableField getField() {
> // What to return here?!

How about TemporaryTable.this.field("ID")?

> }
> };

[Sven Jacobs]

How about TemporaryTable.this.field("ID")? 

 

Duh, sometimes it's too easy ;)

Thank you!

[Sven Jacobs]

How about TemporaryTable.this.field("ID")?  

field() returns a Field but a TableField is required. So I'm still stuck :'(

Would be much easier if TableFieldImpl were public ;-) 

[Lukas Eder]

2013/4/6 Sven Jacobs <sj1...@gmail.com>:

>
>> How about TemporaryTable.this.field("ID")?
>
>
> field() returns a Field but a TableField is required. So I'm still stuck :'(
> Would be much easier if TableFieldImpl were public ;-)

Yeah. It would be easier for a couple of minutes. Then you'd start
hacking around with those internals and everything would explode ;-)

I've updated issue #2374 with a new comment.
https://github.com/jOOQ/jOOQ/issues/2374#issuecomment-16003012

This suggestion might be a solution to your issue where your
QualifiedField isn't equal to the required TableField
Could you please provide some feedback on that?

Cheers
Lukas

[Sven Jacobs]

I encountered the field comparison issue when I "hacked" QualifiedTable to be able to specify an Identity. But I reverted these changes so I cannot test it right now.

Regarding Identity, why must the return value of getField() be a TableField? Identity also has a getTable() so that information is redundant. All usages of getField() that I found either only require a Field or cast the value to Field.

So can we change the return type to Field?

[Lukas Eder]

2013/4/6 Sven Jacobs <sj1...@gmail.com>:

> I encountered the field comparison issue when I "hacked" QualifiedTable to
> be able to specify an Identity. But I reverted these changes so I cannot
> test it right now.
>
> Regarding Identity, why must the return value of getField() be a TableField?
> Identity also has a getTable() so that information is redundant. All usages
> of getField() that I found either only require a Field or cast the value to
> Field.
> So can we change the return type to Field?

Good question. TableField might've been a mistake when it was first
introduced in jOOQ 1.x. The reason is the simple fact that it is very
hard to enforce throughout the API without making those people angry
that do not use the code generator. This can be seen easily by the
fact that the type is hardly ever used in the jOOQ API:
http://www.jooq.org/javadoc/latest/org/jooq/class-use/TableField.html

I will check again, why I hadn't considered removing it in jOOQ 3.0...

Cheers
Lukas

>
> Am Samstag, 6. April 2013 22:04:05 UTC+2 schrieb Lukas Eder:
>>
>> 2013/4/6 Sven Jacobs <sj1...@gmail.com>:
>> >
>> >> How about TemporaryTable.this.field("ID")?
>> >
>> >
>> > field() returns a Field but a TableField is required. So I'm still stuck
>> > :'(
>> > Would be much easier if TableFieldImpl were public ;-)
>>
>> Yeah. It would be easier for a couple of minutes. Then you'd start
>> hacking around with those internals and everything would explode ;-)
>>
>> I've updated issue #2374 with a new comment.
>> https://github.com/jOOQ/jOOQ/issues/2374#issuecomment-16003012
>>
>> This suggestion might be a solution to your issue where your
>> QualifiedField isn't equal to the required TableField
>> Could you please provide some feedback on that?
>>
>> Cheers
>> Lukas
>

[Sven Jacobs]

> Good question. TableField might've been a mistake when it was first
> introduced in jOOQ 1.x. The reason is the simple fact that it is very
> hard to enforce throughout the API without making those people angry
> that do not use the code generator. This can be seen easily by the
> fact that the type is hardly ever used in the jOOQ API:
> http://www.jooq.org/javadoc/latest/org/jooq/class-use/TableField.html
>
> I will check again, why I hadn't considered removing it in jOOQ 3.0...

Changing it to Field would also fix the comparison issue I think because then a Field would be compared to a Field.

Since this is a major release you could argue that API changes are allowed.

[Lukas Eder]

Hi Sven,

And sorry for the delay

2013/4/7 Sven Jacobs <sj1...@gmail.com>

Unfortunately, TableField is used in a couple of places, specifically
the ON KEY join method. I didn't have time to try to resolve that
dependency, but I think it would be possible to pull up the getTable()
method to Field.table() and make it "optional", i.e. allow for it to
return null, if a field's table is unknown or not available.

Cheers
Lukas

[Sven Jacobs]

Hi Lukas,

Unfortunately, TableField is used in a couple of places, specifically
the ON KEY join method. I didn't have time to try to resolve that
dependency, but I think it would be possible to pull up the getTable()
method to Field.table() and make it "optional", i.e. allow for it to
return null, if a field's table is unknown or not available.

if you could do that, that would be great! If you have a better idea of how to fix the initial problem, that's fine with me too ;)

But at the moment I'm using traditional JDBC for an INSERT statement because I need to fetch the generated ID. All other code is jOOQ already. Would really like to completely use jOOQ here.

Thanks

Sven 

[Lukas Eder]

Hi Sven,

Some follow-up on this discussion:

2013/4/14 Sven Jacobs <sj1...@gmail.com>

Hi Lukas,

Unfortunately, TableField is used in a couple of places, specifically
the ON KEY join method. I didn't have time to try to resolve that
dependency, but I think it would be possible to pull up the getTable()
method to Field.table() and make it "optional", i.e. allow for it to
return null, if a field's table is unknown or not available.

if you could do that, that would be great! If you have a better idea of how to fix the initial problem, that's fine with me too ;)

TableField was not removed in jOOQ 3.0. I am still not sure whether we should have that type or not. So I'm leaving it there for now.

Unfortunately, these parts of the jOOQ API are closely coupled with the code-generator's expected output. Fixing these things will take some time, and I'm currently not sure how this can be done.

Have you made any progress on your side?

Cheers

Lukas

 

But at the moment I'm using traditional JDBC for an INSERT statement because I need to fetch the generated ID. All other code is jOOQ already. Would really like to completely use jOOQ here.

Thanks

Sven 

--