Support for casting superclass to inherited class

Issue #648 wontfix
Anonymous created an issue

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)

  1. Michael Bayer 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.

  2. Anonymous

    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>
    
  3. Michael Bayer 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.

  4. Anonymous

    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

  5. Anonymous

    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()
    

    [BR] [BR]

    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

  6. Anonymous

    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

  7. Michael Bayer 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.

  8. Anonymous

    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.

  9. Michael Bayer 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 ? ).

  10. Michael Bayer 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.

  11. Anonymous

    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.

  12. Michael Bayer 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.

  13. Anonymous

    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

  14. Michael Bayer 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.

  15. Michael Bayer repo owner

    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.

  16. Michael Bayer repo owner

    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.

  17. Log in to comment