Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non default Column name causes get_primary_key to fail. #44

Closed
repole opened this issue Dec 5, 2015 · 2 comments

Comments

@repole
Copy link

commented Dec 5, 2015

Say you have a model:

class Author(Base):
    __tablename__ = 'Author'

    author_id = Column("AuthorId", Integer, primary_key=True)
    name = Column("Name", Unicode(160), nullable=False)

get_primary_key will return AuthorId as the primary key, but the attribute that's actually desired is author_id. When I go go to serialize a BookSchema (with a many relationship on authors), the list of Author primary keys incorrectly gets filled with None. Using the below snippet instead solves the issue for me:

from sqlalchemy.orm import ColumnProperty, class_mapper

def get_primary_columns(model):
    """Get primary key columns for a SQLAlchemy model.

    :param model: SQLAlchemy model class
    """
    primary_keys = []
    for primary_key in class_mapper(model).primary_key:
        for prop in class_mapper(model).iterate_properties:
            if isinstance(prop, ColumnProperty):
                if prop.columns:
                    if prop.columns[0].compare(primary_key):
                        primary_keys.append(prop)
                        break
    return primary_keys

Would love some input here on whether this all makes sense.

jmcarp added a commit to jmcarp/marshmallow-sqlalchemy that referenced this issue Dec 7, 2015

Use property names rather than column names for primary keys.
When model properties and table columns have different names, use the
property name rather than the column name. This resolves a bug on
serializing relationships whose primary key columns are aliased to
different names reported by @repole.

[Resolves marshmallow-code#44]

jmcarp added a commit to jmcarp/marshmallow-sqlalchemy that referenced this issue Dec 7, 2015

Use property names rather than column names for primary keys.
When model properties and table columns have different names, use the
property name rather than the column name. This resolves a bug on
serializing relationships whose primary key columns are aliased to
different names reported by @repole.

[Resolves marshmallow-code#44]
@jmcarp

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2015

Makes sense to me--thanks for catching this. I proposed a slightly simpler fix in #45. It's basically the same as your solution, but a little shorter:

def get_primary_keys(model):
    """Get primary key properties for a SQLAlchemy model.

    :param model: SQLAlchemy model class
    """
    mapper = model.__mapper__
    return [
        mapper.get_property_by_column(column)
        for column in mapper.primary_key
    ]

Would you mind checking out the patch and verifying that it resolves the issue you encountered?

@repole

This comment has been minimized.

Copy link
Author

commented Dec 7, 2015

Learn something new with SQLAlchemy every day, didn't realize get_property_by_column existed. Much more graceful solution.

Just tried it out and it works as expected. Thanks!

@sloria sloria closed this in #45 Dec 8, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.