allow_partial_pks=False goes to database on partial pk

[Former user]

See this script, running 0.6.0 (before #1789 fix):

from sqlalchemy import *
from sqlalchemy.orm import *

engine = create_engine('oracle://user:password@localhost:1521/xe?use_ansi=False',echo=True)
metadata = MetaData()
Session = sessionmaker(bind=engine)
session = Session()

order_table = Table("orders", metadata,
   Column("orderid", Unicode, primary_key=True)
)

orderdetail_table = Table("orderdetails",metadata,
   Column("orderid", Unicode, ForeignKey('orders.orderid'),primary_key=True),
   Column("lineid", Integer, primary_key=True),
   Column("saleprice", Numeric, nullable=False)
)

class Order(object):
   pass

class OrderDetail(object):
   pass


order_mapper = mapper(Order, order_table,
       properties=dict(orderdetails=relation(OrderDetail,
                       cascade='all,delete-orphan',
                       single_parent=True,
                       lazy=False,
                       backref=backref('parentorder',
                               cascade='refresh-expire,expunge'))))

orderdetail_mapper = mapper(OrderDetail, orderdetail_table,
       allow_partial_pks=False)


o=Order()
o.orderid = u'SALE000' # not in database

line=OrderDetail()
line.lineid = 1     # not in database

o.orderdetails = [line](line)
merged=session.merge(o)

merged.orderdetails[0](0).saleprice  # <---- referencing this, with allow_partial_pks=False should not go to database

Following is the pasted output, starting with merge():

>>> merged=session.merge(o)
2010-05-06 09:44:49,648 INFO sqlalchemy.engine.base.Engine.0x...5790
SELECT USER FROM DUAL
2010-05-06 09:44:49,652 INFO sqlalchemy.engine.base.Engine.0x...5790
{}
2010-05-06 09:44:49,656 INFO sqlalchemy.engine.base.Engine.0x...5790
BEGIN
2010-05-06 09:44:49,657 INFO sqlalchemy.engine.base.Engine.0x...5790
SELECT orders.orderid AS orders_orderid, orderdetails_1.orderid AS
orderdetails_1_orderid, orderdetails_1.lineid AS
orderdetails_1_lineid, orderdetails_1.saleprice AS
orderdetails_1_saleprice
FROM orders, orderdetails orderdetails_1
WHERE orders.orderid = :param_1 AND orders.orderid =
orderdetails_1.orderid(+)
2010-05-06 09:44:49,657 INFO sqlalchemy.engine.base.Engine.0x...5790
{'param_1': u'SALE000'}
>>> 
>>> merged.orderdetails[0](0).saleprice  # <---- referencing this, with allow_partial_pks=False should not go to database
2010-05-06 09:44:49,664 INFO sqlalchemy.engine.base.Engine.0x...5790
SELECT orderdetails.saleprice AS orderdetails_saleprice
FROM orderdetails
WHERE orderdetails.orderid IS NULL AND orderdetails.lineid = :param_1
2010-05-06 09:44:49,664 INFO sqlalchemy.engine.base.Engine.0x...5790
{'param_1': 1}
>>>

I think this is related to the ticket #1789 in that {{{saleprice}}} shouldn't have been expired on a pending instance anyway.

However, I wanted to report this because it seems to me there is ''also'' potentially something broken with "allow_partial_pks=False" in that, regardless of the expired status, I would have expected the query to not be issued with a pk that is partially None.

[Mike Bayer]

I'm not getting that result on a blank database:

print "----------calling merge---------------"
merged=session.merge(o)
print "----------------------------"
merged.orderdetails[0](0).saleprice  # <--



----------calling merge---------------
2010-05-11 12:09:46,365 INFO sqlalchemy.engine.base.Engine.0x...8f90 BEGIN
2010-05-11 12:09:46,366 INFO sqlalchemy.engine.base.Engine.0x...8f90 SELECT orders.orderid AS orders_orderid, orderdetails_1.orderid AS orderdetails_1_orderid, orderdetails_1.lineid AS orderdetails_1_lineid, orderdetails_1.saleprice AS orderdetails_1_saleprice 
FROM orders LEFT OUTER JOIN orderdetails AS orderdetails_1 ON orders.orderid = orderdetails_1.orderid 
WHERE orders.orderid = ?
2010-05-11 12:09:46,366 INFO sqlalchemy.engine.base.Engine.0x...8f90 (u'SALE000',)
/Users/classic/dev/sqlalchemy/lib/sqlalchemy/engine/base.py:1878: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
  result_processor(dialect, coltype)
----------------------------
z-eeks-Computer-3:sqlalchemy classic$

[Former user]

Sorry, I attempted to make note in the bug description that with #1789 fixed you probably wouldn't get this specific scenario to cause a problem since the pending instance's saleprice is not expired.

However, I wanted to report this because it seems to me there is also potentially something broken with "allow_partial_pks=False" in that, regardless of the expired status, I would have expected the query to not be issued with a pk that is partially None.

Is it the case that this bug cannot be reached with #1789 fixed, or is there still a bug that caused the query to be issued and it is just being masked for you because saleprice isn't expired?

[Mike Bayer]

so am I correct that this ticket is referring to only a potential issue, not one you can actually reproduce ?

[Former user]

''I'' can recreate the problem, but I don't have the fix for #1789 yet.

If you cannot recreate the problem running the same script then off the top of my head I'm not certain whether I could create a scenario either.

I hope you don't feel I am wasting your time. I tried to communicate from the beginning that #1789 would probably fix this particular instance of what looked like incorrect behavior, but it seemed likely it was only masking an underlying problem that allow_partial_pks=False was still trying to fetch from the database on a partial pk.

If you feel it is not worth investigating that suspicion, feel free to close the ticket... I was only trying to be helpful.

Thanks.

[Mike Bayer]

i was only asking if this is something you were observing. there's no public scenario where a pending or transient instance can be marked expired - like I said in #1789 this does occur within the flush process, but only after the instance has been actually flushed in which case it should not have a "partial" pk. so it might be a useful assertion in mapper._load_scalar_attributes as follows:

assert not _none_set.issubset(identity_key) or (mapper.allow_partial_pks and not _none_set.issuperset(identity_key))

[Mike Bayer]

used a warning in f0fa460791735cd27dc41b1ed154465ea2441be5

[Mike Bayer]

Removing milestone: 0.6.1 (automated comment)