Bug #4294
closedSynthetic Primary Key Not Being Reported Correctly
0%
Description
When I make my dbo extend Wt::Dbo::Dbo and call Wt::Dbo::Dbo::id() on it, it returns an incorrect number. On the other hand, when I get the object from the database, Wt::Dbo::ptr::id() returns the correct value. I cooked up a minimum working example: http://sprunge.us/PRRI. My database looks like this: http://sprunge.us/Lead. I compile like this: http://sprunge.us/hjVh , and run like this: http://sprunge.us/VOaF. I'll attach these to this issue to make sure the code still exists in the future.
The output is like this:
getting meals...
begin transaction
select "id", "version", "name" from "meal"
meal
meal
commit transaction
meal first meal has id 22183448 and ptr_id = 1
meal second meal has id 2 and ptr_id = 2
The numbers don't seem to be reliable from Wt::Dbo::Dbo::id(), so it looks like an uninitialized value; If I add another meal or two, the values change randomly.
Wt::Dbo::Dbo::id() is supposed to return the id of the object according to the docs: http://www.webtoolkit.eu/wt/doc/reference/html/classWt_1_1Dbo_1_1Dbo.html#ae1f77ea4a99f84e0e7561001c5e6d1fb "Returns the database id of this object, or Wt::Dbo::dbo_traits::invalidId() if the object is associated with a session or not yet stored in the database." (note that the odd value I'm seeing I don't think is invalidId=--1).
Files
Updated by Koen Deforche almost 9 years ago
- Status changed from New to Feedback
- Assignee set to Koen Deforche
Hey,
You're copying the 'meal' object by value. That's not something that's supported. Since I don't know how that should behave we'll probably make the Wt::Dbo<> copy constructor private.
Koen
Updated by lm at almost 9 years ago
Oh, interesting. I haven't looked at the code, but would it be difficult to add copy and move constructors?
Updated by lm at almost 9 years ago
I might as well ask about copy and move assignment operators, too :-) They should either be =delete or defined properly while we're at it.
Updated by Koen Deforche almost 9 years ago
Hey,
What if you have two different C objects representing the same database object. How should that behave? Should a change in one be reflected in the other? In the database version? etc... The main assumption of an ORM is that an in-memory copy of a persisted object (e.g. which has a database counterpart) serves as a kind of 'cache'/'proxy' towards the database object. It's not clear how to extend this to multiple in-memory copies of the same object (within a single session).
Koen
Updated by lm at almost 9 years ago
I can buy that, thanks. I deleted my object's copy ctor, and Wt::Dbo::Dbo's should probably =delete, too :-). While we're here, I guess we should make sure that move semantics work as they ought. I'll try to test that out soon, and report back.
Updated by lm at almost 9 years ago
It looks like the hangman example does the same thing in Session.C:179 ? (It copies a Wt::Dbo::Dbo.)
Updated by lm at almost 9 years ago
I did a bit of work on getting an object extending Wt::Dbo::Dbo out of a Wt::Dbo::ptr, and putting it into a vector. I'm uploading a new cpp;
In the business object, I have defined:
C::C()=default and C::C (C&&)=default; which leaves C::C (C const &) deleted (by omission; see rule of 3). Then, it's necessary to use .modify() to get a non-const ptr, and std::move() to make sure the copy constructor isn't invoked. It looks like there is no UB (uninitialized values uncovered).
Updated by Koen Deforche almost 9 years ago
Hey,
Hangman seems indeed guilty of that and isn't setting the good example. I've instead decided to leave the copy constructor, but properly initialize the meta data, so that self() will return a null ptr in the copy.
Disabling the copy operation all together is a bit too restrictive for a generic base class.
Regards,
Koen
Updated by Koen Deforche almost 9 years ago
- Status changed from Feedback to Resolved
Updated by lm at almost 9 years ago
I almost disagree with leaving the copy constructor but creating copies that don't act the same as the original. I agree that disabling copy might be heavy-handed, but I guess a note in the docs explaining the behaviour would perhaps be as good as we can expect. Thanks!
Updated by Koen Deforche over 8 years ago
- Status changed from Resolved to Closed