JOOQ update primary key using .store()

[Ryan How]

Hi,

Is there a way to override the default behaviour of creating a new record where the primary key has been updated?. I would like to update the primary key using .store(). I was thinking.... if the original value of the primary key was null and then it was changed, then it should insert. If the value was not null and it was changed, it should update.

Don't want to get into a primary key update debate, I pretty much never update primary keys, but I have a special case :)

Anyway, I don't see an issue with optionally changing the behaviour of store()? :)

Thanks, Ryan

[Lukas Eder]

If you have a special case whose violation of normalisation principles
we won't discuss now ;-), you should use the regular jOOQ API for
updating records. For instance:
http://www.jooq.org/javadoc/latest/org/jooq/impl/Factory.html#executeUpdate(R)

> If the value
> was not null and it was changed, it should update.

The semantics put into store() on that occasion is documented in the
Javadoc. Due to the normalisation principles, changing the primary key
value can only mean wanting to create a copy of this record. On the
other hand, Record.copy() does precisely that, but more explicitly.

> Anyway, I don't see an issue with optionally changing the behaviour of
> store()? :)

I'm reluctant to modify that store() too much. It has too many
"clever" features already. On the other hand, its behaviour has been
challenged on this user group time and again. Maybe there is room for
discussion about a potential change in jOOQ 3.0

According to you, when store()ing a record, which was previously
loaded from the database, whose primary key value was modified in
Java, then the record should be updated as such:
UPDATE [table] SET [pk] = [new value], ... WHERE [pk] = [original value]

Note, the "original value" of any Record value is only available since
jOOQ 2.6.0:
https://github.com/jOOQ/jOOQ/issues/1849

Any other opinions on this subject?

[Ryan How]

According to you, when store()ing a record, which was previously
loaded from the database, whose primary key value was modified in
Java, then the record should be updated as such:
UPDATE [table] SET [pk] = [new value], ... WHERE [pk] = [original value]

Yep, to put it in a nutshell :). Otherwise I would expect to do record.copy() if I wanted to create a new record.

Small example. If I did
Record r = factory.... fetch();
r.setFieldXValue(someNewValue);
r.store();

I would expect it to update, no matter what field was changed (so if it was the primary key or not it shouldn't matter)

If I did

Record r = factory.... fetch();
Record r2 = r.copy();
r2.setFieldXValue(someNewValue);
r2.store();

Then have a new record.


But as changing it was break existing code which "clones" a record without explicitly calling copy(), I would propose to have a setting which indicates what behaviour should happen on a primary key update when using .store()

[Ryan How]

I'm reluctant to modify that store() too much. It has too many
"clever" features already.

I think doing this would actually make it "dumber", because it would care if a field was a primary key or not, if it is changed it will update. To create a new record, .copy() should be called first.

[Lukas Eder]

> I would expect it to update, no matter what field was changed (so if it was
> the primary key or not it shouldn't matter)

The more I think of it, the more I agree. Such a change would go well
with the Principle of Least Astonishment:
http://en.wikipedia.org/wiki/Principle_of_least_astonishment

> But as changing it was break existing code which "clones" a record without
> explicitly calling copy(), I would propose to have a setting which indicates
> what behaviour should happen on a primary key update when using .store()

Or, since jOOQ 3.0 is a major release, maybe we should just change
this behaviour backwards-incompatibly. I could then add some remarks
to the jOOQ 2.x maintenance version Javadoc, indicating that this
behaviour will be rectified in 3.0. I have created #1949 for this:
https://github.com/jOOQ/jOOQ/issues/1949

I will generally re-think the INSERT / UPDATE behaviour of
Record.store() in jOOQ 3.0, keeping as much backwards-compatibility as
possible, of course. But this topic has lead to confusion many times
on this user group, so it's worth re-thinking

Cheers
Lukas

[Lukas Eder]

Yes. In software engineering, "dumb" (= very predictable) is often
better than "clever"... I explicitly used the term "clever" here (as
opposed to "elegant"), which is always a sign of smelly code / design.

[Durchholz, Joachim]

> Any other opinions on this subject?

It can break all kinds of assumptions if modifying the PK is allowed.

People tend to use PK->Entity HashMaps when dealing with large-ish quantifies of records. These will break as soon as the PK is modified.
This is not a problem as long as the programmer that does the PK update also does the HashMap, but sometimes entities get passed around and kept track of behind the scenes, and those behind-the-scenes mechanisms will break.

That said, I think there should be a way to update PKs, but it should be clearly marked as dangerous. Whether that's a big fat warning in store's javadoc, or a special updateThePk function, or anything else, I can't say.

One data point from the outside: Hibernate is using a cache of load-time field values as the basis for a considerable fraction of its services, and the keys are PK values. If you modify a PK, Hibernate will fail, crash and burn, and very quickly.
This in itself isn't necessarily relevant to JOOQ, but it's building programmer expectations, and also something that you need to explain to people coming from Hibernate to JOOQ.
(I don't know how other ORMs handle the issue, but I wouldn't be surprised if they did the same.)

[Lukas Eder]

Hi Joachim,

Thanks for sharing your thoughts.

>> Any other opinions on this subject?
>
> It can break all kinds of assumptions if modifying the PK is allowed.

Yes. Modifying PK values is very bad practice. This is independent of
jOOQ, JDBC, Java, and any other database access layer. It is bad from
a relational perspective.

> People tend to use PK->Entity HashMaps when dealing with large-ish quantifies of records. These will break as soon as the PK is modified.
> This is not a problem as long as the programmer that does the PK update also does the HashMap, but sometimes entities get passed around and kept track of behind the scenes, and those behind-the-scenes mechanisms will break.

Quite right. Once a PK is changed, all sorts of dependent objects need
to be updated.

> That said, I think there should be a way to update PKs, but it should be clearly marked as dangerous. Whether that's a big fat warning in store's javadoc, or a special updateThePk function, or anything else, I can't say.

Good idea. If store() is changed, its Javadocs will contain such a warning.

Nevertheless, just avoiding PK changes and instead performing Record
copying might be even more weird to some users. If some client code
changes a PK value by accident (i.e. through a bug), then running an
INSERT is about as bad as "accidentally" updating the PK.

> One data point from the outside: Hibernate is using a cache of load-time field values as the basis for a considerable fraction of its services, and the keys are PK values. If you modify a PK, Hibernate will fail, crash and burn, and very quickly.

I feel that the whole database should fail, crash and burn

Cheers
Lukas

[Ryan How]

> Good idea. If store() is changed, its Javadocs will contain such a warning.
>
> Nevertheless, just avoiding PK changes and instead performing Record
> copying might be even more weird to some users. If some client code
> changes a PK value by accident (i.e. through a bug), then running an
> INSERT is about as bad as "accidentally" updating the PK.

You could just throw an exception on PK update.

In my case, I can use a system generated PK and then set my current PK
as unique field instead. I have no records linking to this table. So it
would just mean redundant data in my case which is also violating
relational database principles :)... well not really redundant, but a
useless field just for the purpose of not changing the primary key.

In any case I don't think JOOQ should stop a user updating the PK. If
they want to then they can sort out their own mess later.

Let's talk about updating a composite primary key! :)

[Ryan How]

On 7/11/2012 7:17 PM, Ryan How wrote:
> You could just throw an exception on PK update.

Just thinking some more. I think this might be a better option.

Imagine if the PK has been updated in-between the fetch and store by
another transaction. Then this could have disastrous behaviour. And it
would break optimistic locking.

[Ryan How]

I am now updating my table to add a system generated PK :)

[Durchholz, Joachim]

>> One data point from the outside: Hibernate is using a cache of
>> load-time field values as the basis for a considerable fraction
>> of its services, and the keys are PK values. If you modify a PK,
>> Hibernate will fail, crash and burn, and very quickly.
>
> I feel that the whole database should fail, crash and burn

Hehe, I sympathize with that sentiment. If databases made PKs read-only, we'd also finally have an easily explainable difference between PKs (read-only) and unique keys (updatable).
That's not gonna happen I guess *dreams*.

A general observation: altering a PK is like renaming something. For some kinds of records, the name is the identity; for others, name and identity can be seen as independent. (Using a separate artificial numeric id is then the "real identity" if you will.)

Allowing PK changes is definitely going to kill optimistic locking. You don't even have a clearcut idea what "the same record" even means in that context: If another process changes the PK to yet another value, should that be considered a copy or a conflicting update? I guess the answer depends on the situation, which means that JOOQ would need to be told which one is right. Meaning "more API", which is a downside.

PK changes start to get a stable semantics if the record is locked on SELECT. Then the identity of the records won't change while you're busy updating the PK.
Maybe that's the way to go: Allow PK updates if and only if the object came by means of SELECT ... FOR UPDATE or some other means of locking. Of course, that's not optimistic locking anymore, but I don't think the whole concept is very useful in a PK-modifying situation.

Disclaimer:
I'm just tossing around ideas based on general DB experience, recent Hibernate experience, and a very thin varnish of JOOQ knowledge from reading the docs. Once. Diagonally.
IOW just tell me as soon as I'm starting to go off on a tangent that's irrelevant to JOOQ, so I'll shut up before making a complete fool of myself ;-)

[Lukas Eder]

> You could just throw an exception on PK update.

I'm not so sure about that. The rest of the jOOQ API wouldn't throw
such an exception either. E.g., you could always explicitly update the
PK using Factory.update() or Factory.executeUpdate(), or plain SQL...

> Imagine if the PK has been updated in-between the fetch and store by
> another transaction. Then this could have disastrous behaviour. And it would
> break optimistic locking.

Some databases will create a table lock when you update a primary key.
If not, you should SELECT ... FOR UPDATE such a record, if you intend
to change the primary key.

Anyway, best practices are not relevant for jOOQ (apart from the
Javadoc, maybe), just as they are not relevant for JDBC. "If it's
possible with SQL, it should be possible with jOOQ"...

Cheers
Lukas

[Lukas Eder]

> If databases made PKs read-only, we'd also finally have an easily explainable difference between PKs (read-only) and unique keys (updatable).

Yes, that would be quite useful. I don't see any other reason to
distinguish PKs and UKs.

> PK changes start to get a stable semantics if the record is locked on SELECT. Then the identity of the records won't change while you're busy updating the PK.
> Maybe that's the way to go: Allow PK updates if and only if the object came by means of SELECT ... FOR UPDATE or some other means of locking. Of course, that's not optimistic locking anymore, but I don't think the whole concept is very useful in a PK-modifying situation.

Precisely. You should pessimistically lock the row when your intent is
changing the PK. And you should lock it as early as possible.

Some more interesting insight on the topic:
http://stackoverflow.com/a/3838649/521799

> Disclaimer:
> I'm just tossing around ideas based on general DB experience, recent Hibernate experience, and a very thin varnish of JOOQ knowledge from reading the docs. Once. Diagonally.
> IOW just tell me as soon as I'm starting to go off on a tangent that's irrelevant to JOOQ, so I'll shut up before making a complete fool of myself ;-)

No worries. These things are not off-topic on this user group.

Cheers
Lukas