UpdatableRecords wrong detection of value change

53 views
Skip to first unread message

Daniele Antonini

unread,
Oct 23, 2013, 5:09:31 AM10/23/13
to jooq...@googlegroups.com
Hi,

I'm playing a bit with UpdatableRecords, and I'm falling into this issue: primary key is marked as changed also when setted value is the same of prevoius one.

I wirte a test to reproduced the issue:
1) using set<columnName> of generated record
2) using ModelMapper (corresponds to the previous case because it uses generated set)

This test fails:
@Test public void test() {
    AccountUserRecord r = ctx.fetchOne(ACCOUNT_USER, ACCOUNT_USER.ID.eq(1L));
    r.setId(1L);
    assertFalse(r.changed(ACCOUNT_USER.ID));
}

When I use setValue(Field, value) the behaviour is correct, as a matter of fact this test pass:
@Test public void test() {
    AccountUserRecord r = ctx.fetchOne(ACCOUNT_USER, ACCOUNT_USER.ID.eq(1L));
    r.setValue(ACCOUNT_USER.ID, 1L);
    assertFalse(r.changed(ACCOUNT_USER.ID));
}

For completeness' sake I attached a full maven project to highlight the problem, the project require mysql.

Generated AccountUserRecord.setId is the following code:
public void setId(java.lang.Long value) {
setValue(0, value);
}

Why use setValue(int, value) instead of setValue(Field, value)?
Is this an issue of code generator?

PersonRecord p = //... select ...
assert p.getName() == "Otto";
p.setName("Max"); //ok changed is true
p.setName("Max"); // ok changed is still true, change flag is not resetted
p.setName("Otto"); //why changed flag remains true? 
p.store();

Cheers

--
Daniele


UpdatableRecordTest.zip

Lukas Eder

unread,
Oct 23, 2013, 11:01:43 AM10/23/13
to jooq...@googlegroups.com
Hi Daniele,

The rationale behind this is easy to explain:
PersonRecord p = //... select ...
assert p.getName() == "Otto";
p.setName("Otto"); //why changed flag remains true? 
p.store();
Explicitly setting a value in a record means that you want this value to appear in an INSERT / UPDATE statement. This may have desired side-effects on triggers and other database objects. Note that there is also


p.reset();

In order to reset the changed flags and the value.

More comments inline:

2013/10/23 Daniele Antonini <antonini...@gmail.com>



Hi,

I'm playing a bit with UpdatableRecords, and I'm falling into this issue: primary key is marked as changed also when setted value is the same of prevoius one.



I wirte a test to reproduced the issue:
1) using set<columnName> of generated record
2) using ModelMapper (corresponds to the previous case because it uses generated set)



This test fails:
@Test public void test() {
    AccountUserRecord r = ctx.fetchOne(ACCOUNT_USER, ACCOUNT_USER.ID.eq(1L));


    r.setId(1L);
    assertFalse(r.changed(ACCOUNT_USER.ID));


}

When I use setValue(Field, value) the behaviour is correct, as a matter of fact this test pass:
@Test public void test() {


    AccountUserRecord r = ctx.fetchOne(ACCOUNT_USER, ACCOUNT_USER.ID.eq(1L));
    r.setValue(ACCOUNT_USER.ID, 1L);


    assertFalse(r.changed(ACCOUNT_USER.ID));
}



Hmm, yes there's a bug in the above behaviour. The two calls are not exactly equivalent, which is not good. I'll investigate this:
 
For completeness' sake I attached a full maven project to highlight the problem, the project require mysql.



Generated AccountUserRecord.setId is the following code:

	public void setId(java.lang.Long value) {
		setValue(0, value);


	}

Why use setValue(int, value) instead of setValue(Field, value)?



Internally, records use field indexes, not Field references. So this call is a bit more performant than setValue(Field, value). But the logic should still be the same (which will be investigated in #2798)



Hope this helps
Cheers
Lukas
 


Is this an issue of code generator?



PersonRecord p = //... select ...
assert p.getName() == "Otto";
p.setName("Max"); //ok changed is true
p.setName("Max"); // ok changed is still true, change flag is not resetted
p.setName("Otto"); //why changed flag remains true? 
p.store();

Cheers



--
Daniele








-- 

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.





Daniele Antonini
unread,
Oct 23, 2013, 1:23:56 PM10/23/13
to jooq...@googlegroups.com
Lukas,

On Wed, Oct 23, 2013 at 5:01 PM, Lukas Eder <lukas...@gmail.com> wrote:




Hi Daniele,

The rationale behind this is easy to explain:







PersonRecord p = //... select ...
assert p.getName() == "Otto";





p.setName("Otto"); //why changed flag remains true? 
p.store();
Explicitly setting a value in a record means that you want this value to appear in an INSERT / UPDATE statement. This may have desired side-effects on triggers and other database objects. Note that there is also






p.reset();

In order to reset the changed flags and the value.
Thanks, now it's clear.

In my domain, I have no trigger, nor other database objects. I need only to store data into database, and I want to do this in efficient way and reduce database sql size may help.




For my needed the best is to set only values that are effectively updated, since I use ModelMapper I'd rather have my domain object, map it to one or more records and invoke store(). 
I don't want to worry to know which fields are changed.





I have implemented this beaviour with the following class:

public class SetOnlyChangedValueAccountUserRecord implements IAccountUser {




  private static boolean equals(final Object orig, final Object current) {
    if (orig == current) { return true; }




    if (orig != null && current != null) { return orig.equals(current); }
    return false;
  }




  private IAccountUser delegate;
  private IAccountUser orig;





  public SetOnlyChangedValueAccountUserRecord(final AccountUserRecord r) {
    delegate = r;
    orig = r.original();




  }
  @Override
  public void setEmail(final String value) {




    if (!equals(orig.getEmail(), value)) {
      delegate.setEmail(value);
    }




  }
  //other set, get , and other stuff
}





Same result can be obtained with a proxy class, using a bit of reflection, to make this "solution" general purpose.
Do you think such utility class should be part of JOOQ, or developed when needed from developer?
 Ok





Hope this helps 




Cheers
Lukas
 






Is this an issue of code generator?







PersonRecord p = //... select ...
assert p.getName() == "Otto";
p.setName("Max"); //ok changed is true
p.setName("Max"); // ok changed is still true, change flag is not resetted
p.setName("Otto"); //why changed flag remains true? 
p.store();

Cheers






--
Daniele




Cheers
-- 
Daniele



Lukas Eder
unread,
Oct 25, 2013, 11:03:13 AM10/25/13
to jooq...@googlegroups.com
Hi Daniele,


2013/10/23 Daniele Antonini <antonini...@gmail.com>
I'm not sure if your proxy is correct. Let's assume this control flow:



assertEquals("x", user.getEmail());


user.setEmail("x"); // This has no effect


user.setEmail("y"); // This sets email to "y"


user.setEmail("x"); // This has no effect (!)


I think you shouldn't compare the new value with orig.getEmail() value but with delegate.getEmail() instead. Or even better, this:



     delegate.setEmail(value);
     if (equals(orig.getEmail(), value)) {


       delegate.changed(ACCOUNT_USER.EMAIL, false);
     }

Also, calls to user.setValue(ACCOUNT_USER.EMAIL, "y") are not intercepted. This might be irrelevant if you only expose the interface that contains setters and getters, not the jOOQ record.



I agree that it might be nice to be able to influence whether assigning the same value again to a Record should set the changed flag to true. I think that this behaviour should be adaptable through a Setting, though. There is a feature request pending for jOOQ 3.3:



Cheers
Lukas
 





-- 
Daniele Antonini
unread,
Oct 25, 2013, 12:16:34 PM10/25/13
to jooq...@googlegroups.com
Lukas,

 You're right, I have blundered :)
 






I agree that it might be nice to be able to influence whether assigning the same value again to a Record should set the changed flag to true. I think that this behaviour should be adaptable through a Setting, though. There is a feature request pending for jOOQ 3.3:





Ok, happy to be useful




Cheers
 





-- 

You received this message because you are subscribed to a topic in the Google Groups "jOOQ User Group" group.

To unsubscribe from this topic, visit https://groups.google.com/d/topic/jooq-user/gtMxNzg02NA/unsubscribe.

To unsubscribe from this group and all its topics, send an email to jooq-user+...@googlegroups.com.


For more options, visit https://groups.google.com/groups/opt_out.





-- 
Daniele



Lukas Eder
unread,
Dec 7, 2013, 6:40:33 AM12/7/13
to jooq...@googlegroups.com, Daniele Antonini
This issue could now be fixed for jOOQ 3.3.0:



It will be merged to 3.2.2. It has also helped reproduce another, somewhat related issue:



Cheers
Lukas


2013/10/25 Daniele Antonini <antonini...@gmail.com>
Reply all
Reply to author
Forward
0 new messages