Eager load with several column properties with the same label fail

Issue #1019 resolved
Former user created an issue

(original reporter: ged) Ok, I'm not sure at all this is a defect but it's a bit annoying because of several reasons.

First, it used to work before r4467. Secondly, I relied on this behavior in Elixir: I'm labeling each column_property with the name of the attribute. This is just because I found it quite annoying to have to label each column_property manually for no apparent reason (it's not as if the user cared what it'd be labeled to), and the column_property required a labeled construct. Now this blows up when an user manages to fetch several attributes with the same label in the same query via an eager load, as seen in the attached test case (which worked fine before r4467).

On a related note: column_property() on a select without a label doesn't work anymore. Should I open a separate bug report for this? Update: grrrr. It seems like the unlabeled selectable does work in pure SQLAlchemy... So that part is either an Elixir-only issue or a bug which only show up through Elixir because of the order it initialize stuff.

Now, what I suggest is either not to care at all about the manually-defined label of constructs in a column_propery (I don't see the point in manually defining them anyway), or/and fix the "unlabeled" selectable in column_property, so that I can remove altogether the automatic label generation in Elixir code.

IMO, at the minimum, SA should issue a warning (or even an exception) that there are two constructs using conflicting labels, instead of silently returning wrong values.

Comments (3)

  1. Mike Bayer repo owner
    • changed milestone to 0.4.xx
    • changed component to orm

    right, removing the usage of .label() altogether fixes it, since SA generates the labels as needed during compilation. (theres a slight chance that this might change someday if we move towards positional column targeting...but I don't think im going to get into that for awhile).

    But also, i think what you're looking for here is this (basically, the equivalent):

    tag_score = (tags_table.c.score1 * tags_table.c.score2).label(None)
    
    mapper(Tag, tags_table, properties={
        'query_score': column_property(tag_score),
    })
    
    user_score = select([*
                                  tags_table.c.score2)](func.sum(tags_table.c.score1),
                        tags_table.c.user_id == users_table.c.id) \
                        .label(None)
    

    Its even safe for you to take in these selects and just label then with .label(None) even if they already have a label.

    as far as two columns being named the same in a SQL, that's valid; someone might be selecting and just using positional targeting to get at the columns.

    anyway the particular case here is the eager load is not anonymizing the label, this would be a feature add. It only worked prior to e3b2305d6721a1f1ed20f9c520765f7c33876f32 due to a bug which was fixed. The feature itself looks like this:

    Index: lib/sqlalchemy/orm/util.py
    ===================================================================
    --- lib/sqlalchemy/orm/util.py  (revision 4592)
    +++ lib/sqlalchemy/orm/util.py  (working copy)
    @@ -165,6 +165,8 @@
    
             # process column-level subqueries    
             aliased_column = sql_util.ClauseAdapter(self.alias, equivalents=self.equivalents).traverse(column, clone=True)
    +        if isinstance(aliased_column, expression._Label):
    +            aliased_column = aliased_column.label(None)
    
             # add to row decorator explicitly
             self.row_decorator({}).map[column](column) = aliased_column
    

    So I'll leave this ticket open until I can get test coverage in for the above patch and also the 0.5 equivalent.

  2. Log in to comment