reverse inheriting column order for 0.7

[Mike Bayer]

this was the original idea:

--- a/lib/sqlalchemy/orm/mapper.py  Wed Aug 25 11:01:46 2010 -0400
+++ b/lib/sqlalchemy/orm/mapper.py  Wed Aug 25 15:55:01 2010 -0400
@@ -625,7 +625,7 @@
                     # existing ColumnProperty from an inheriting mapper.
                     # make a copy and append our column to it
                     prop = prop.copy()
-                prop.columns.append(column)
+                prop.columns.insert(0, column)
                 self._log("appending to existing ColumnProperty %s" % (key))
             elif prop is None or isinstance(prop, ConcreteInheritedProperty):
                 mapped_column = []

lots of tests had to be changed with this

makes things more intuitive with the "id" column and such. very few real test failures, and those tests seem to be doing abusive things anyway.

this would need very extensive testing and very comprehensive expectation of what surprises this would cause upon user upgrade.

[Mike Bayer]

Getting all the tests to work with this revealed a disturbing, slightly unrelated issue:

    @testing.resolve_artifact_names
    def test_mapping_to_join(self):
        """Mapping to a join"""
        usersaddresses = sa.join(users, addresses,
                                 users.c.id == addresses.c.user_id)
        mapper(User, usersaddresses, primary_key=[users.c.id](users.c.id))
        l = create_session().query(User).order_by(users.c.id).all()
        eq_(l, self.static.user_result[:3](:3))

above, the mapping is relationally incorrect. But it "works" because "user.id" is first. Flipping that order silently breaks the mapping as intended, and its actually extremely difficult to try to guess if this is a situation we should be warning on, and its also super difficult to not map addresses.id to the same attribute - exclude_properties is against column names, not columns, so it would be very tough for a new user to figure out what's going on here.

[Mike Bayer]

the attached patch flips around how the function is implemented. We leave the order of columns the same and instead return columns-1 for the expression only, and nothing else. This allows a bunch of tests that assert SQL to stay the same, and the order of columns queried is preserved more like that of the selectables that are mapped (also allowing the above gotcha to act the same, though its still wrong, and querying by User.id will produce the wrong result) Also in the patch is the use case that came up on the mailing list regarding this, though I've hit this one myself all over the place.

It's not clear if this is the best approach. In cases where the two columns are different (a user error condition), now the attribute will reference one value, and the expression column another.

[Mike Bayer]

the next patch is the original way, with the mapping order reversed, and the tests changed to reflect it.

Another idea is a third way, where inheritance gets involved and explicitly places the "local" table's column first. Since we are relying on some indirect decisionmaking here. Not sure.

[Mike Bayer]

its just one line of code but the ramifications are really quite time consuming here and I'm already two hours into it.

[Mike Bayer]

related to #1896 which is partially implemented for 0.6

[Mike Bayer]

007344a40a9ec8f1abed3af1ff7c1473031f7b50

[Mike Bayer]

Removing milestone: 0.7.0 (automated comment)