- attached teste.py
Support for casting superclass to inherited class
I could not find a way to do that and I think it's an enhancement request.
Would be really nice to have a way to cast between classes. I mean, when you have Person and Employee classes, for example (where Employee is a subclasse of Person), would be great to create an Employee based on a previous saved Person.
Today, if you try to do:
>>> # myPerson = a Person object from database
>>> myPerson.__class__ = Employee
>>> myPerson.working = True
>>> session.save(myPerson)
>>> session.flush()
you would get an identity change Exception:
Can't change the identity of instance Employee@-0x4873a7d4 in session (existing identity: (<class '__main__.Person'>, (1,), None); new identity: (<class '__main__.Employee'>, (1,), None))
(see attached script for details)
Would be possible to do something like this?
Comments (26)
-
Account Deleted -
repo owner you can do this now if you add:
# remove session idenity del jose._instance_key # remove primary key attribute del jose.id session.save(jose) # ...
id favor a method session.resave() that does this.
-
Account Deleted - attached modified_teste.py
The modified test case
-
Account Deleted Not working.
When deleting the _instance_key and the ID attributes, it's creating a new ID for that object in database, duplicating the hole row.
See above execution and attached modified test script.
tfrazao@sharon ~/Desktop $ python teste.py 2007-07-12 19:17:58,244 INFO sqlalchemy.engine.base.Engine.0x..74 CREATE TABLE person ( id INTEGER NOT NULL, name VARCHAR(255), PRIMARY KEY (id) ) 2007-07-12 19:17:58,245 INFO sqlalchemy.engine.base.Engine.0x..74 None 2007-07-12 19:17:58,283 INFO sqlalchemy.engine.base.Engine.0x..74 COMMIT 2007-07-12 19:17:58,287 INFO sqlalchemy.engine.base.Engine.0x..74 CREATE TABLE employee ( id INTEGER NOT NULL, working BOOLEAN, PRIMARY KEY (id), FOREIGN KEY(id) REFERENCES person (id) ) 2007-07-12 19:17:58,288 INFO sqlalchemy.engine.base.Engine.0x..74 None 2007-07-12 19:17:58,297 INFO sqlalchemy.engine.base.Engine.0x..74 COMMIT 2007-07-12 19:17:58,303 INFO sqlalchemy.engine.base.Engine.0x..74 BEGIN 2007-07-12 19:17:58,306 INFO sqlalchemy.engine.base.Engine.0x..74 INSERT INTO person (name) VALUES (?) 2007-07-12 19:17:58,307 INFO sqlalchemy.engine.base.Engine.0x..74 ['Jose']('Jose') 2007-07-12 19:17:58,309 INFO sqlalchemy.engine.base.Engine.0x..74 COMMIT 2007-07-12 19:17:58,317 INFO sqlalchemy.engine.base.Engine.0x..74 BEGIN 2007-07-12 19:17:58,319 INFO sqlalchemy.engine.base.Engine.0x..74 INSERT INTO person (name) VALUES (?) 2007-07-12 19:17:58,320 INFO sqlalchemy.engine.base.Engine.0x..74 ['Jose']('Jose') 2007-07-12 19:17:58,341 INFO sqlalchemy.engine.base.Engine.0x..74 INSERT INTO employee (id, working) VALUES (?, ?) 2007-07-12 19:17:58,342 INFO sqlalchemy.engine.base.Engine.0x..74 [1](2,) 2007-07-12 19:17:58,344 INFO sqlalchemy.engine.base.Engine.0x..74 COMMIT tfrazao@sharon ~/Desktop $ sqlite3 /tmp/teste.sqlite SQLite version 3.3.17 Enter ".help" for instructions sqlite> select * from person; 1|Jose 2|Jose sqlite> select * from employee; 2|1 sqlite>
-
repo owner no, we dont really support that...that would have to combine an INSERT with an UPDATE for the inheritance conversion and would be an involved operation for an very rare use case. youre best off deleting the Person, creating a copy of it, and removing the _instance_key off the copy. the delete()/save() operation, with the same id attribute, will perform the switch.
-
Account Deleted That's what I've been doing. Making a copy, deleting the old object and creating a new one (with a new ID), but, for performance reasons, it's not a good idea to do that if the parent class/table has many relations.
But thanks anyway. May I send you a patch if I find some time to change it?
Thiago F Pappacena
-
Account Deleted Hey! :D
It was not that hard at all...
I have a version that seems to work fine for me (and for test/alltests.py ;)).
I created a cast(instance, Subclass) method in session object. It's using python's copy module to make a copy of the object and mark it to save, returning the 'casted' object. See an example:
>>> # jose = you know who I'm talking about >>> jose = session.cast(jose, Employee) >>> jose.working = True >>> session.flush()
I'm adding the patch as an attached file. Just do a
patch -p0 < sqlalchemy_cast.patch
inside SQLAlchemy's trunk directory and everything might be fine.
I would be really happy to see it in next release of SQLAlchemy :)
[]'s! [BR] Thiago F Pappacena
-
Account Deleted - attached sqlalchemy_cast.patch
The patch to session.cast() method
-
Account Deleted - attached sqlalchemy_cast.2.patch
Sorry. Last upload was for the wrong patch file
-
repo owner what happens if I cast from "Employee" back to "Person" ? is it going to delete the row from the "employee" table ?
-
Account Deleted humm... not by now. This patch casts only TO subclass, but no way back FROM subclass (casting back will raise an AssertationError). A little selfish, but I needed this to solve my problem. :$
I will work on it to find a way to cast back to Person and send you the patch, ok?
Just a question: who can commit to trunk? Is there a way to give me access to commit there?
Thiago F Pappacena
-
repo owner i can commit to trunk as well as a few other people here. note that we try to add as few features like this, i.e. things that you can do manually with more explicitness, as possible to the core of SA...for this to be useful, it has to work in all expected cases and be very simple...I already dont like the "extra flags" being attached to the object here as far as the conditional logic ...it amounts to a "if very rare use case X then Y; else z" style of programming which increases complexity and reduces test coverage. i am still skeptical of this feature so far.
-
Account Deleted It's not that easy to do manually. And it's impossible to do without a explicit "INSERT INTO child_table (id) VALUES (%(parent_id)d)", which is not a good pratice if you have a ORM, right?
Deleting the superclass object to save again if you have related objects to it is a realy, realy poor pratice, since SA will need to do lots of DELETEs/INSERTs with no good reason. The DBA will kill us all for doing that! ;)
Subclass casting is not that rare case. You mapped a inheritance relation and your model was good. Now, the client just ask you "I need to make this <insert here you generalized class> a <insert here you especialized class>. It's possible?". Would you say "No, because I need to put one flag and an 'if' in my code."?
And I added only one flag (_sa_casted_from) to track casting, and it's removed after session.flush(). I agree that another 'if' for special cases are not that good, but I see a lot of harmless "if not hasattrs(x, 'y'): continue" in SA today.
-
repo owner Replying to guest:
Subclass casting is not that rare case. You mapped a inheritance relation and your model was good. Now, the client just ask you "I need to make this <insert here you generalized class> a <insert here you especialized class>. It's possible?". Would you say "No, because I need to put one flag and an 'if' in my code."?
thats a restatement of what youd like to do, but does not explain why you'd want to do this. If I knew the scenario where this was actually needed, that would help to define what we are really trying to do here, and how this method should be presented...as it stands, it would be just an arbitrary method that nobody would ever use since its not clear at all why anyone needs to "change the class" of an object, instead of the more obvious activity of just creating an Employee explicitly if thats what one wishes to save. this is not something ive ever seen any ORM support, including Hibernate (does ActiveRecord support it ? ).
-
repo owner since i take every feature request seriously, ive referred this ticket over on the sqlalchemy-devel list on google groups, so that we can discuss how this feature should look, if implemented.
-
Account Deleted Ok... it's fair to put it to discussion. Can you send me the URL of the thread?
Why not just create a new Employee and drop the old Person? 'Cause you would need to drop all relations of that Person and re-create them. Imagine that a Person has relation to Email, Address, Group, Dependents, Food, EscolarityLevel...
Drop the Person is not that desired if it has relations to all these other classes, cause it's easy to write the code to just re-create another identical object, but it's too expensive to a DBMS to perform several DELETEs/INSERTs to copy something that is alreary there.
Now that you talked about, every List/Vector class in Java.util uses the language suport to 'subclassing' objects.
Vector v = new Vector(2); MyClass a = new MyClass(); v.addElement(a); System.out.println("Without casting: " + v.get(0).doSomething()); // raise compiling exception (cannot find symbol 'doSomething' for java.lang.Object instance) System.out.println("After casting: " + ((MyClass) v.get(0)).doSomething()); // works fine
I don't know if Hibernate can do this cast, but trust me: people in Java world that really knows about OO would like it a lot.
-
repo owner OK, ive been convinced that this is a valid use case (but not because of the hibernate example; i still dont think hibernate supports this operation). and yes, it would be a delete of the Person switched with a new Employee, but not issue a DELETE on the underlying table - its turned to an UPDATE.
this feature already exists, however it does not work across inheritance changes, which would auto-detect additional/fewer tables to be additionally INSERT/DELETEd. this would require changes to mapper.save_obj(), specifically within the logic that detects "is_row_switch" to also determine the status of additional tables that change based on the specific mapper used. it also does not account for an inheritance relationship where subclasses define composite primary keys and base classes do not.
so i have to milestone this as 0.5 for now, which means "anytime between 0.4 and 0.5". i have a lot that needs to go out with 0.4 already. "needs a volunteer" means, if someone wants to work on it thats fine.
-
Account Deleted Changing the INSERT to an UPDATE was done in mapper.save_obj() (see second patch file). Not in "is_row_switch", but in another variable (called "isinsert") with the same logic:
1099 isinsert = not instance_key in uowtransaction.uow.identity_map and not postupdate and not has_identity(obj) 1099 # if the object was casted from another one, do not insert into the actual table if it's from the base object. Do an update! 1100 if hasattr(obj, '_sa_casted_from') and table in object_mapper(obj._sa_casted_from).tables: 1101 isinsert = False 1102 else: 1103 isinsert = not instance_key in uowtransaction.uow.identity_map and not postupdate and not has_identity(obj)
I just need to adapt to superclass casting, i.e., from Employee to Person class.
I know you don't like the '_sa_casted_from' extra flag (neither do I), but any suggestion to detect the cast in mapper.save_obj()?
Thiago F Pappacena
-
Account Deleted (original author: ged)
-
Account Deleted - assigned issue to
(original author: ged) Hell, I didn't want to assign it to myself...
-
repo owner Replying to guest:
I know you don't like the '_sa_casted_from' extra flag (neither do I), but any suggestion to detect the cast in mapper.save_obj()?
it will do a delete()/save() of the old/new objects, the copy operation can be done whereever. the is_row_switch logic will be expanded to look at the mapped tables of the previous and the new mappers, and calculate the insert/delete/update status of each table involved.
-
repo owner - changed milestone to 0.5.xx
-
repo owner - changed milestone to 0.6.xx
-
repo owner - marked as minor
-
repo owner - changed milestone to blue sky
I'm really not seeing this feature getting added unless a core committer wanted to take it on fully. Tests would need to support the full range of operations, including INSERT/DELETE of related tables. It would be quite complicated and it would also be super nice if it could be a recipe or feature outside of the core of the ORM, possibly using events.
-
repo owner - changed status to wontfix
there's no interest in this feature, as for the extremely rare case that someone wants to modify a joined inheritance identity into a different one, they should emit the requisite insert() / delete() manually; would be very complicated to do here.
- Log in to comment
A script that can exemplify the request