Issues

Issue #2828 resolved

emit warning when Column is assigned directly to multiple names

jerryji
created an issue

column_property hides original column from mapper when used with no change.

Results from running the attached column_property.py, tested against sqlalchemy git clone just now --

(sqlalchemy) C:\sqlalchemy>ipython
Python 2.7.5 (default, May 15 2013, 22:44:16) [v.1500 64 bit (AMD64)](MSC)

In [1](1): import sqlalchemy
In [2](2): sqlalchemy.__version__
Out[2](2): '0.9.0'

In [3](3): %run column_property.py
['id']('name_alias',)
['id', 'name']('name_alias',)

column_property.py (in case attachment fails), maybe not the right way to create an aliasing column, but still looks like a bug to me --

from sqlalchemy import (
    create_engine,
    Column,
    Text,
    Integer,
)
from sqlalchemy.orm import (
    scoped_session,
    sessionmaker,
    column_property,
)
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class NameMissing(Base):
    __tablename__ = 'name_missing'
    id = Column(Integer, primary_key=True)
    name = Column(Text)
    name_alias = column_property(name)

    def __init__(self, id, name):
        self.id = id
        self.name = name

class NameOK(Base):
    __tablename__ = 'name_ok'
    id = Column(Integer, primary_key=True)
    name = Column(Text)
    name_alias = column_property(name+'')

    def __init__(self, id, name):
        self.id = id
        self.name = name

engine = create_engine('sqlite://')
Base.metadata.create_all(engine)
dbsession = scoped_session(sessionmaker(bind=engine))
new_name_missing = NameMissing(1, 'foobar')
dbsession.add(new_name_missing)
new_name_ok = NameOK(1, 'foobar')
dbsession.add(new_name_ok)
dbsession.commit()

name_missing = dbsession.query(NameMissing).first()
print name_missing.__mapper__.columns.keys()

name_ok = dbsession.query(NameOK).first()
print name_ok.__mapper__.columns.keys()

Jerry

Comments (3)

  1. Mike Bayer repo owner

    there's no bug here, the declaration of name = Column is equivalent of name_alias = column_property(Column), and an explicit column property trumps the plain column as you might be passing extra mapping options along. We can emit a warning for when this occurs, this patch is a start, needs some cleanup and tests:

    diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
    index 820c087..73723bc 100644
    --- a/lib/sqlalchemy/ext/declarative/base.py
    +++ b/lib/sqlalchemy/ext/declarative/base.py
    @@ -14,7 +14,7 @@ from ... import util, exc
     from ...sql import expression
     from ... import event
     from . import clsregistry
    -
    +import collections
    
     def _declared_mapping_info(cls):
         # deferred mapping
    @@ -173,15 +173,19 @@ def _as_declarative(cls, classname, dict_):
    
         # extract columns from the class dict
         declared_columns = set()
    +    col_to_prop = collections.defaultdict(dict)
         for key, c in list(our_stuff.items()):
             if isinstance(c, (ColumnProperty, CompositeProperty)):
                 for col in c.columns:
                     if isinstance(col, Column) and \
                         col.table is None:
                         _undefer_column_name(key, col)
    +                    if isinstance(c, ColumnProperty):
    +                        col_to_prop[col](col)[key](key) = c
                         declared_columns.add(col)
             elif isinstance(c, Column):
                 _undefer_column_name(key, c)
    +            col_to_prop[c](c)[key](key) = c
                 declared_columns.add(c)
                 # if the column is the same name as the key,
                 # remove it from the explicit properties dict.
    @@ -190,6 +194,10 @@ def _as_declarative(cls, classname, dict_):
                 # in multi-column ColumnProperties.
                 if key == c.key:
                     del our_stuff[key](key)
    +    for col, entry in col_to_prop.items():
    +        if len(entry) > 1:
    +            util.warn("Column object named directly multiple times, "
    +                "only one will be used: %s" % (", ".join(entry)))
         declared_columns = sorted(
             declared_columns, key=lambda c: c._creation_order)
         table = None
    diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
    index 4336c19..f828180 100644
    --- a/lib/sqlalchemy/orm/mapper.py
    +++ b/lib/sqlalchemy/orm/mapper.py
    @@ -1218,7 +1218,6 @@ class Mapper(_InspectionAttr):
                 self._log("Identified primary key columns: %s", primary_key)
    
         def _configure_properties(self):
    -
             # Column and other ClauseElement objects which are mapped
             self.columns = self.c = util.OrderedProperties()
    

    this also should probably try to detect if a straight Column is specified multiple times.

    in this use case you want to be using synonym in order to produce a straight "alias" of a name.

  2. Log in to comment