Re: [sqlalchemy] session merge sets missing children foreign keys to null

2021-12-09 Thread Gabriel Smith

Thank you for the quick and clear answer that solves the test case.

I've applied the same approach to the issue in our current codebase and it 
completely fixed the issue.

You're the best!
On Wednesday, December 8, 2021 at 1:48:45 PM UTC-6 Mike Bayer wrote:

>
>
> On Wed, Dec 8, 2021, at 10:38 AM, Gabriel Smith wrote:
>
> Hi, I'm a recent adopter of sqlalchemy, starting fresh with all the 2.0 
> stuff. Thanks so much for the entire teams hard work!
>
>
> great!
>
>
> I have a small issue with the merge functionality and it could honestly 
> just be a misunderstanding of the function from my own point of view. I 
> have a fairly complicated database structure that with 15 nested children 
> tables, some of which have thousands of read-only records. I'd prefer to 
> not have to add all those records to the session in order to run a merge, 
> but when I try to do that, the subsequent flush emits sql updates that set 
> the child foreign keys to null.  
>
> If there is a way to avoid those updates for orphaned children and just 
> ignore them if they aren't included in the incoming data by primary key, 
> that would really help me, if not, I can look into adding them all to the 
> incoming entity so they'll be ignored.
>
>
> yeah this is happening because of this line:
>
>incoming_item.item_prices.append(incoming_price1)
>
> this means you are merging the item like this:
>
>   Item(item_prices = [ItemPrice(...)])
>
> noting that the other two ItemPrice objects are no longer in that 
> collection, which is why they are seen as a net remove.Merge doesn't 
> add an incoming collection to the one which is already there, it sees the 
> collection coming in as "this is the new collection" and replaces the old 
> collection with the new one, so it would need to still have the other two 
> objects present within it, either as the same persistent prices you already 
> have or as additional transient objects with the correct primary key.
> Below we can do it in the former style if we use a future style Session .
>
> This is not a simple case to figure out, as you want to get those price 
> objects from persistent and re-merge them, all the while not tripping up 
> and accidentally adding the transient "incoming_item" into the session.   
> this is how we can do it:
>
> incoming_item.id = persisted_item.id
> 
> # first copy persisted_item's item_prices collection
> incoming_item.item_prices = persisted_item.item_prices
> 
> # then append to that
> incoming_item.item_prices.append(incoming_price1)
>
> but with "legacy" ORM behavior, this will get confused and raise an error 
> since it will try to cascade "incoming_item" into the Session prematurely 
> and then try to flush it.   There's a way to work around that with 
> make_transient, but more succinctly, if we turn on 2.0 behavior for the 
> Session, this unwanted cascade is prevented:
>
> Session = sessionmaker(bind=engine, future=True)
>
> that way there isn't even that much overhead to re-merging those existing 
> prices since they are already persistent in that session.
>
> As you gave me a full, runnable script, I was able to make those 
> adjustments and your assertion at the end succeeds.
>
> great job with the MCVE and paying close attention to the docs as well as 
> making our jobs easier!   
>
>
>
> MCVE:
> ```
> from sqlalchemy import Column, Integer, String, ForeignKey, create_engine, 
> select
> from sqlalchemy.orm import registry, declarative_base, relationship, 
> sessionmaker
> from sqlalchemy.sql.sqltypes import Numeric
> import unittest
>
> Base = declarative_base()
> Mapper_Registry = registry()
>
> ### MODELS
>
> @Mapper_Registry.mapped
> class Item:
> __tablename__ = 'item'
>
> id = Column(Integer, *primary_key*=True)
> model_number = Column(String)
>
> item_prices = relationship("Item_Price", *back_populates*="item", 
> *lazy*="joined")
>
> @Mapper_Registry.mapped
> class Item_Price:
> __tablename__ = 'item_price'
>
> id = Column(Integer, *primary_key*=True)
> item_id = Column(Integer, ForeignKey('item.id'))
> price = Column(Numeric)
>
> item = relationship("Item", *back_populates*="item_prices", 
> *lazy*="joined", 
> *viewonly*=True)
>
> ### TESTS
>
> class Test_OrphanRecordFKMerge(unittest.TestCase):
> engine = create_engine('sqlite:///:memory:', *echo*=True, 
> *echo_pool*='debug', 
> *future*=True)
> Session = sessionmaker(*bind*=engine)
> session = Session()
>
> def setUp(*self*):
> Base2 = Mapper_Registry.generate_base()
> Base2.metadata.create_all(self.engine)
> # Create a base item to run tests on
> t_item = Item()
> t_item.model_number = 'TestItem'
> t_price1 = Item_Price()
> t_price1.price = 1.00
> t_item.item_prices.append(t_price1)
> t_price2 = Item_Price()
> t_price2.price = 4.00
> t_item.item_prices.append(t_price2)
>
> 

Re: [sqlalchemy] session merge sets missing children foreign keys to null

2021-12-08 Thread Mike Bayer


On Wed, Dec 8, 2021, at 10:38 AM, Gabriel Smith wrote:
> Hi, I'm a recent adopter of sqlalchemy, starting fresh with all the 2.0 
> stuff. Thanks so much for the entire teams hard work!

great!

> 
> I have a small issue with the merge functionality and it could honestly just 
> be a misunderstanding of the function from my own point of view. I have a 
> fairly complicated database structure that with 15 nested children tables, 
> some of which have thousands of read-only records. I'd prefer to not have to 
> add all those records to the session in order to run a merge, but when I try 
> to do that, the subsequent flush emits sql updates that set the child foreign 
> keys to null.  
> 
> If there is a way to avoid those updates for orphaned children and just 
> ignore them if they aren't included in the incoming data by primary key, that 
> would really help me, if not, I can look into adding them all to the incoming 
> entity so they'll be ignored.

yeah this is happening because of this line:

   incoming_item.item_prices.append(incoming_price1)

this means you are merging the item like this:

  Item(item_prices = [ItemPrice(...)])

noting that the other two ItemPrice objects are no longer in that collection, 
which is why they are seen as a net remove.Merge doesn't add an incoming 
collection to the one which is already there, it sees the collection coming in 
as "this is the new collection" and replaces the old collection with the new 
one, so it would need to still have the other two objects present within it, 
either as the same persistent prices you already have or as additional 
transient objects with the correct primary key.Below we can do it in the 
former style if we use a future style Session .

This is not a simple case to figure out, as you want to get those price objects 
from persistent and re-merge them, all the while not tripping up and 
accidentally adding the transient "incoming_item" into the session.   this is 
how we can do it:

incoming_item.id = persisted_item.id

# first copy persisted_item's item_prices collection
incoming_item.item_prices = persisted_item.item_prices

# then append to that
incoming_item.item_prices.append(incoming_price1)

but with "legacy" ORM behavior, this will get confused and raise an error since 
it will try to cascade "incoming_item" into the Session prematurely and then 
try to flush it.   There's a way to work around that with make_transient, but 
more succinctly, if we turn on 2.0 behavior for the Session, this unwanted 
cascade is prevented:

Session = sessionmaker(bind=engine, future=True)

that way there isn't even that much overhead to re-merging those existing 
prices since they are already persistent in that session.

As you gave me a full, runnable script, I was able to make those adjustments 
and your assertion at the end succeeds.

great job with the MCVE and paying close attention to the docs as well as 
making our jobs easier!   


> 
> MCVE:
> ```
> from sqlalchemy import Column, Integer, String, ForeignKey, create_engine, 
> select
> from sqlalchemy.orm import registry, declarative_base, relationship, 
> sessionmaker
> from sqlalchemy.sql.sqltypes import Numeric
> import unittest
> 
> Base = declarative_base()
> Mapper_Registry = registry()
> 
> ### MODELS
> 
> @Mapper_Registry.mapped
> class Item:
> __tablename__ = 'item'
>
> id = Column(Integer, *primary_key*=True)
> model_number = Column(String)
> 
> item_prices = relationship("Item_Price", *back_populates*="item", 
> *lazy*="joined")
> 
> @Mapper_Registry.mapped
> class Item_Price:
> __tablename__ = 'item_price'
>
> id = Column(Integer, *primary_key*=True)
> item_id = Column(Integer, ForeignKey('item.id'))
> price = Column(Numeric)
> 
> item = relationship("Item", *back_populates*="item_prices", 
> *lazy*="joined", *viewonly*=True)
> 
> ### TESTS
> 
> class Test_OrphanRecordFKMerge(unittest.TestCase):
> engine = create_engine('sqlite:///:memory:', *echo*=True, 
> *echo_pool*='debug', *future*=True)
> Session = sessionmaker(*bind*=engine)
> session = Session()
> 
> def setUp(*self*):
> Base2 = Mapper_Registry.generate_base()
> Base2.metadata.create_all(self.engine)
> # Create a base item to run tests on
> t_item = Item()
> t_item.model_number = 'TestItem'
> t_price1 = Item_Price()
> t_price1.price = 1.00
> t_item.item_prices.append(t_price1)
> t_price2 = Item_Price()
> t_price2.price = 4.00
> t_item.item_prices.append(t_price2)
> 
> self.session.add(t_item)
> self.session.commit()
> 
> def tearDown(*self*):
> Base.metadata.drop_all(self.engine)
> 
> def test_item_update(*self*):
> self.session.expunge_all()
> # Incoming item data from remote api or flat file
> incoming_item = Item()
> incoming_item.model_number = 'TestItem'
>