mappedcollection converter is obsolete, conflicts with bulk_replace

[sqlalchemy-bot]

Migrated issue, originally created by Michael Bayer (@zzzeek)

e.g. make setitem and set symmetric; if users want validation that would be a separate event. partial patch forthcoming

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm.collections import attribute_mapped_collection

Base = declarative_base()


class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)
    bs = relationship("B", collection_class=attribute_mapped_collection("key"))


class B(Base):
    __tablename__ = 'b'
    id = Column(Integer, primary_key=True)
    key = Column(String)
    a_id = Column(ForeignKey('a.id'))

    def __repr__(self):
        return "B(%s)" % id(self)

a1 = A()

b1 = B(key='foo')
b2 = B(key='foo')


a1.bs['foo'] = b1
a1.bs['bar'] = b2

print a1.bs

a1 = A()
a1.bs = {"foo": b1, "bar": b2}

print a1.bs

---

Attachments: 3604.patch

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• attached file 3604.patch

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

https://gerrit.sqlalchemy.org/52

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

this is a subtle issue not making much impact, moving out

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• edited description

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed milestone from "1.2" to "1.3"

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• removed labels: feature
• added labels: bug

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed title from "remove auto-key-validation from mappedcollection b" to "auto-key-validation in mappedcollection bulk set i"

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

so the patch here is obsolete given that we have now done #3896 e.g. 9974e9a, will have to be adapted

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed title from "auto-key-validation in mappedcollection bulk set i" to "auto-key-validation in mappedcollection bulk set i"

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

not sure how do to do it yet

https://gerrit.sqlalchemy.org/#/c/zzzeek/sqlalchemy/+/883

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

I think it's not really worth changing. The single-item __setitem__ is really what's doing the wrong thing, by allowing the incompatible key to come in. The architecture does not easily allow for __setitem__ to be validating however and it would be a major change to alter this and also would probably break compatibility with user-defined mapped dictionaries. The bulk set is actually more "correct", so we'll just have it emit a warning and live with the inconsistency.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Additionally, the "bulk_replace" event receives only the values and not the keys - doing a fully key-wise bulk set here would require changing the event interface. But again, it's to set the dictionary to a state that by definition can't be persisted, since keys have to be deriveable from the values.

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed title from "auto-key-validation in mappedcollection bulk set i" to "auto-key-validation in mappedcollection bulk set i"

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

can't really emit a warning either as the "converter" is getting in the way of being able to use the bulk_replace event with values that we want to convert. "converter" is basically obsolete.

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed title from "auto-key-validation in mappedcollection bulk set i" to "mappedcollection converter is obsolete conflicts w"

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Remove MappedCollection converter; deprecate @converter

Removed the collection converter used by the :class:.MappedCollection
class. This converter was used only to assert that the incoming dictionary
keys matched that of their corresponding objects, and only during a bulk set
operation. The converter can interfere with a custom validator or
:meth:.AttributeEvents.bulk_replace listener that wants to convert
incoming values further. The TypeError which would be raised by this
converter when an incoming key didn't match the value is removed; incoming
values during a bulk assignment will be keyed to their value-generated key,
and not the key that's explicitly present in the dictionary.

Overall, @converter is superseded by the
:meth:.AttributeEvents.bulk_replace event handler added as part of
🎫3896.

Fixes: #3604
Change-Id: Id0f7bd2cec938f5975eb2ab94df9ba5754dd43c3

→ fe8ddb7

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed