Updating existing records

[digulla]

I've run into trouble updating existing records which I've stored in a cache.

Background: I fetch some records from the database and then convert them to POJOs to make sure no references to the factory/connection/datasource are kept alive in the cache. This means the PK column is copied.

When I want to update such a records, by creating a new record and updating it's columns with the values from the POJO, I get a PK collision because jOOQ assumes that this is a new row (because I have to call the setter for the PK column).

Looking at the code, I haven't found a way to convert a POJO to a record and get an UPDATE during store(). Is there one? I'd rather avoid reading the record again from the DB to update columns.

Regards,

A. Digulla

[Lukas Eder]

Hello,

> Background: I fetch some records from the database and then convert them to
> POJOs to make sure no references to the factory/connection/datasource are
> kept alive in the cache. This means the PK column is copied.

If you're using the POJO only for "detaching" records from their
underlying connections, I might have a workaround for you:

- You could serialise / deserialise the Result / Record objects. As
the underlying connection is transient, it will not be restored.
- Another option is to use Record.attach() to "detach" records manually:
http://www.jooq.org/javadoc/latest/org/jooq/Attachable.html#attach(org.jooq.Configuration)

I wonder whether there should be a Setting flag to indicate that jOOQ
should not automatically "attach" resulting records? While this
automatic attachment is convenient for simple use-cases, it's
obviously prohibitive when connection and memory-management are
important...

> Looking at the code, I haven't found a way to convert a POJO to a record and
> get an UPDATE during store(). Is there one? I'd rather avoid reading the
> record again from the DB to update columns.

How are you loading the POJO into the record? Using Factory.newRecord()?
http://www.jooq.org/javadoc/latest/org/jooq/impl/Factory.html#newRecord(org.jooq.Table,
java.lang.Object)

As far as I recall, that will indeed set all "changed" flags in the
record's values to true, resulting in an INSERT to be performed. I
wonder what would be a sensible way to indicate to jOOQ, that a record
loaded from a POJO should be considered as an UPDATE candidate, rather
than an INSERT candidate... Any ideas?

Cheers
Lukas

[Lukas Eder]

> I wonder whether there should be a Setting flag to indicate that jOOQ
> should not automatically "attach" resulting records? While this
> automatic attachment is convenient for simple use-cases, it's
> obviously prohibitive when connection and memory-management are
> important...

This is tracked as #1684:
https://github.com/jOOQ/jOOQ/issues/1684

[digulla]

Am Donnerstag, 9. August 2012 12:21:44 UTC+2 schrieb Lukas Eder:

- You could serialise / deserialise the Result / Record objects. As
the underlying connection is transient, it will not be restored.

That would work but I'm wary of the performance :-)

 

- Another option is to use Record.attach() to "detach" records manually:
  http://www.jooq.org/javadoc/latest/org/jooq/Attachable.html#attach(org.jooq.Configuration)

Can you please update the documentation to say how to *de*tach a record? Can I call attach(null)?

 

I wonder whether there should be a Setting flag to indicate that jOOQ
should not automatically "attach" resulting records? While this
automatic attachment is convenient for simple use-cases, it's
obviously prohibitive when connection and memory-management are
important...

In my case, I'm generating "smart" POJOs (which contains some business logic), so any changes in the record types wouldn't help.

 

How are you loading the POJO into the record? Using Factory.newRecord()?
http://www.jooq.org/javadoc/latest/org/jooq/impl/Factory.html#newRecord(org.jooq.Table,
java.lang.Object)

record.updateFrom( pojo );

This is a new method which is generated by my modified code generator that copies every field.

 

As far as I recall, that will indeed set all "changed" flags in the
record's values to true, resulting in an INSERT to be performed. I
wonder what would be a sensible way to indicate to jOOQ, that a record
loaded from a POJO should be considered as an UPDATE candidate, rather
than an INSERT candidate... Any ideas?

Many:

- An update() and insert() method on the record or a parameter in the store() method.

- Make the code in org.jooq.impl.AbstractRecord.into(Table<R>) which resets the PK available in a public method

- Add a field which says whether to INSERT or UPDATE. If it's not set or set to AUTO, use the field flags.

I'm undecided which one would be the best approach.

Regards,

A. Digulla

[Lukas Eder]

>> - You could serialise / deserialise the Result / Record objects. As
>> the underlying connection is transient, it will not be restored.
>
> That would work but I'm wary of the performance :-)

Yes, it's not a very elegant workaround

>> - Another option is to use Record.attach() to "detach" records manually:
>>
>> http://www.jooq.org/javadoc/latest/org/jooq/Attachable.html#attach(org.jooq.Configuration)
>
> Can you please update the documentation to say how to *de*tach a record? Can
> I call attach(null)?

I will:
https://github.com/jOOQ/jOOQ/issues/1685

>> I wonder whether there should be a Setting flag to indicate that jOOQ
>> should not automatically "attach" resulting records? While this
>> automatic attachment is convenient for simple use-cases, it's
>> obviously prohibitive when connection and memory-management are
>> important...
>
> In my case, I'm generating "smart" POJOs (which contains some business
> logic), so any changes in the record types wouldn't help.

OK, I see. Well, it might still be useful to some users...

>> How are you loading the POJO into the record? Using Factory.newRecord()?
>>
>> http://www.jooq.org/javadoc/latest/org/jooq/impl/Factory.html#newRecord(org.jooq.Table,
>> java.lang.Object)
>
> record.updateFrom( pojo );
>
> This is a new method which is generated by my modified code generator that
> copies every field.

How is it different from this one?
http://www.jooq.org/javadoc/latest/org/jooq/Record.html#from(java.lang.Object)

>> [...] Any ideas?

>
> Many:
>
> - An update() and insert() method on the record or a parameter in the
> store() method.

That probably makes sense anyway. I have added feature request #1686 for this:
https://github.com/jOOQ/jOOQ/issues/1686

I prefer adding update() and insert() over store() argument flags for
two reasons:

- There is a risk of adding dozens of flags, eventually
- They already exist, internally

> - Make the code in org.jooq.impl.AbstractRecord.into(Table<R>) which resets
> the PK available in a public method

I'd like to keep these things hidden from the public API. Relying on
them will make it difficult to evolve jOOQ's CRUD API, which has quite
a bit of complexity already.

> - Add a field which says whether to INSERT or UPDATE. If it's not set or set
> to AUTO, use the field flags.

I assume you mean adding a field to Record? That would be redundant
with the "changed" info of the PK, although more expressive in its
intent.

> I'm undecided which one would be the best approach.

Among these three, clearly the first one. I'll think about it a bit
more. The question here is whether the decision of updating/inserting
should be done

1. At the moment when store() / insert() / update() is called
2. At the moment when a POJO is loaded into a record

I'd like to explore some options around scenario 2, as well...

Cheers
Lukas

[Lukas Eder]

>> Can you please update the documentation to say how to *de*tach a record? Can
>> I call attach(null)?
>
> I will:
> https://github.com/jOOQ/jOOQ/issues/1685

N.B: I've checked the integration tests. The most succinct way to
detach a record is to attach null to it:

store = create.newRecord(TBookStore());
store.setValue(TBookStore_NAME(), "Barnes and Noble");
store.attach(null);

try {
store.store();
fail();
}
catch (DetachedException expected) {}

Cheers
Lukas

[digulla]

Am Donnerstag, 9. August 2012 17:00:07 UTC+2 schrieb Lukas Eder:

>> How are you loading the POJO into the record? Using Factory.newRecord()?

> record.updateFrom( pojo );
> This is a new method which is generated by my modified code generator that
> copies every field.

How is it different from this one?
http://www.jooq.org/javadoc/latest/org/jooq/Record.html#from(java.lang.Object)

It doesn't use reflection. Since I know all the fields and getters and setters, it's easy to generate a method that just calls all of them:

/**

* Copy the values of this record into another instance

*/

public void copyTo(com.avanon.blu.jooq.gen.tables.interfaces.adm.ILocation other) {

other.setPK(getPK());

other.setXmlImportIdentifier(getXmlImportIdentifier());

                ...

}

/**

* Copy the values of another instance into this record

*/

public void updateFrom(com.avanon.blu.jooq.gen.tables.interfaces.adm.ILocation other) {

setXmlImportIdentifier(other.getXmlImportIdentifier());

                ...

}

 

>> [...] Any ideas?
> Many:
> - An update() and insert() method on the record or a parameter in the
> store() method.

That probably makes sense anyway. I have added feature request #1686 for this:
https://github.com/jOOQ/jOOQ/issues/1686

I prefer adding update() and insert() over store() argument flags for
two reasons:

- There is a risk of adding dozens of flags, eventually
- They already exist, internally

> - Add a field which says whether to INSERT or UPDATE. If it's not set or set
> to AUTO, use the field flags.

I assume you mean adding a field to Record? That would be redundant
with the "changed" info of the PK, although more expressive in its
intent.

Maybe it would make sense to combine all columns related to the PK into a PK Java type, so the whole PK gets a single "has changed" flag.

But I have a feeling that this could cause nasty corner cases when someone tries to manipulate the individual columns with setValue(field, ...);

 

Among these three, clearly the first one. I'll think about it a bit
more. The question here is whether the decision of updating/inserting
should be done

1. At the moment when store() / insert() / update() is called
2. At the moment when a POJO is loaded into a record

I'd like to explore some options around scenario 2, as well... 

Well, Hibernate solves this like so: If the PK is null, the identity generator is started to assign a PK (well, I simplify but you get the idea) and an INSERT will happen. If the PK is not null, it will UPDATE.

This feels natural because it uses the assumption that you can't have a PK unless the record has been read from the DB. And if you have a PK, then the DB must already know about it.

So maybe a good solution would be to modify jOOQ to support a similar behavior. With my suggested changes in the Configuration class, it would be simple to add the PK generator to it. Everything else should then be a purely internal change of the logic.

Regards,

Aaron

[Lukas Eder]

>> How is it different from this one?
>>
>> http://www.jooq.org/javadoc/latest/org/jooq/Record.html#from(java.lang.Object)
>
>
> It doesn't use reflection. Since I know all the fields and getters and

> setters, it's easy to generate a method that just calls all of them: [...]

That is a nice idea. Care to contribute? :-)
In essence, the code-generator could do this:

- If records AND pojos are generated, it would generate into(POJO) and
from(POJO) methods
- If records AND interfaces are generated, it would generate
into(IFace) and from(IFace) methods

The second rule would override the first. I'll file this as feature
request #1689
https://github.com/jOOQ/jOOQ/issues/1689

Besides, from your logic, I'd say that adding a general <E> E
Record.into(E) method might be a useful addition, copying data into
previously instanciated custom POJO types:
https://github.com/jOOQ/jOOQ/issues/1688

> Maybe it would make sense to combine all columns related to the PK into a PK
> Java type, so the whole PK gets a single "has changed" flag.

I have thought about this before. Having multi-column primary keys
modelled as a single object would help resolve some API issues that
can currently not be resolved - e.g. generated DAOs don't really
support multi-column PKs.

Adding such PK support might look simple at first, but the
corresponding foreign keys might also have to be modelled accordingly,
I guess. I'm not sure yet, if I want to go down that road. Anyway,
let's track the idea as #1690:
https://github.com/jOOQ/jOOQ/issues/1690

> But I have a feeling that this could cause nasty corner cases when someone
> tries to manipulate the individual columns with setValue(field, ...);

The PK Java object would have to encapsulate the same behaviour
today's org.jooq.impl.Value internal object encapsulates. I.e.
changing one PK column would mean that the whole PK can be considered
as "changed". This is already the case today, although the information
is distributed across multiple org.jooq.impl.Value objects.
Record.setValue() has all the necessary information to update any PK's
internal "changed" flags, should a PK member be changed individually
through other API methods. With a sufficient number of test cases, I
don't think that would be a problem.

> Well, Hibernate solves this like so: If the PK is null, the identity
> generator is started to assign a PK (well, I simplify but you get the idea)
> and an INSERT will happen. If the PK is not null, it will UPDATE.

Yes, I know this behaviour by Hibernate. In the early days of jOOQ, I
have evaluated it against other options.

> This feels natural because it uses the assumption that you can't have a
> PK unless the record has been read from the DB. And if you have a PK,
> then the DB must already know about it.

This model might suit 90% of user needs. But jOOQ also accomodates
those 10% who generate PK values in Java (using UUID's, explicit
Oracle sequence calls, etc.). In those cases, the PK values are set
explicitly upon a Record with the intent of executing an INSERT, not
an UPDATE.

In other words, you *can* have a PK value that the DB does not yet
know about. The only way to be sure that the PK is already in the DB
is when the Record was actually fetched from the DB.

Now, of course, this leads to the inverse problem that you, as a user,
cannot set a PK value upon a (new) Record, without jOOQ assuming that
you intend to INSERT this Record.

> So maybe a good solution would be to modify jOOQ to support a similar behavior.

I prefer not to change that logic at this stage. It might break quite
a few applications.

> With my suggested changes in the Configuration class, it would be simple
> to add the PK generator to it. Everything else should then be a purely
> internal change of the logic.

I'm still exploring alternatives of supporting PK generators in
Record.store() calls. I also like the idea to support a central
authority for this in the Configuration / Settings objects. In fact, I
would like to go much farther and support something like an
org.jooq.Trigger type, that can be configured centrally in the
Configuration. This trigger type could then be parameterised to listen
to any type of statement event. I would like to get inspiration from
standard SQL triggers, which feature:

- Scope (i.e. listening to all events, or just to events for some tables)
- Statement type (i.e. listening to any statement, or just inserts/updates, etc)
- Conditions (i.e. triggered always, or just when some condition is met)
- Before / After
- etc.

Such triggers would be implemented as ExecuteListeners.

Let's track this, too:
https://github.com/jOOQ/jOOQ/issues/1691

Cheers
Lukas

[Lukas Eder]

> Again, if my primary key is a user name, an email, a region or some shipping
> code, I'd be unable to insert...

True. In some degrees of relational normalisation, this is quite expected.

> I'd prefer having a factory method with some DSL such as
> TAuthorRecord author = Factory.updateRecord(T_AUTHOR).wherePk( AuthorId );

In a less fluent way, this is already possible today:

--------------------------------------------------
UpdateQuery update = Factory.updateQuery(T_AUTHOR);
update.setRecord(author);
update.addConditions(T_AUTHOR.ID.equal(author.getId());
update.execute();

// or...

Factory.executeInsert(T_AUTHOR, author);
Factory.executeUpdate(T_AUTHOR, author, T_AUTHOR.ID.equal(author.getId()));
--------------------------------------------------

The above API originates from very early days of jOOQ. In the mean
time, I guess there is some redundancy in it (e.g. having to pass the
table). I guess there is some room for improvement there. I'll track
such improvements as #1692:
https://github.com/jOOQ/jOOQ/issues/1692

> rather than letting some obscure logic decide.
> Compared to record-level methods like markAsNew(), storeAsUpdate, storeAsNew
> this would not confuse internal states, dirty fields tracking and such.

Well, it might still be useful to actually add Record.insert() and
Record.update() methods, in case users do want to reset dirty fields
tracking.

[digulla]

Am Freitag, 10. August 2012 11:45:22 UTC+2 schrieb (unbekannt):

Well, Hibernate solves this like so: If the PK is null, the identity generator is started to assign a PK (well, I simplify but you get the idea) and an INSERT will happen. If the PK is not null, it will UPDATE.

I guess it would work if the PK is based on a sequence, but it is not uncommon to have non auto-increment PKs, how would hibernate do it in this case?

When store() is called, Hibernate notices that the PK is NULL (or 0L if you use long) and it remembers that bit of information. Depending on the type of PK generator, it will either generate a new PK and store it in the field (from this point on, H. doesn't need the PK anymore - it has a boolean flag which tells it whether to UPDATE or INSERT) or it will store the record with a NULL PK and read the new PK from the DB after the INSERT and put that into the field.

 

This feels natural because it uses the assumption that you can't have a PK unless the record has been read from the DB. And if you have a PK, then the DB must already know about it.

Again, if my primary key is a user name, an email, a region or some shipping code, I'd be unable to insert...

I always run into problems with natural PKs, so I'm not using them.

Regards,

A. Digulla

[Lukas Eder]

>>> How is it different from this one?
>>>
>>> http://www.jooq.org/javadoc/latest/org/jooq/Record.html#from(java.lang.Object)
>>
>> It doesn't use reflection. Since I know all the fields and getters and
>> setters, it's easy to generate a method that just calls all of them: [...]
>

> In essence, the code-generator could do this:
>
> - If records AND pojos are generated, it would generate into(POJO) and
> from(POJO) methods
> - If records AND interfaces are generated, it would generate
> into(IFace) and from(IFace) methods
>
> The second rule would override the first. I'll file this as feature
> request #1689
> https://github.com/jOOQ/jOOQ/issues/1689

#1689 is now implemented on Github master and will be included in jOOQ
3.0, possibly in 2.7 as well. It allows for typesafe loading of record
data from/into POJOs, Records, as long as interfaces are generated