On Thu, 1 May 2008 12:20:39 -0400 Michael Bayer <[EMAIL PROTECTED]> wrote: > > at first the second method seemed a little hacky to me, but now that > i think about it, we're in the sharded query so removing the extra > entity is not a big deal. The first method is probably not as > fragile as you think, as if someone uses MyClass.someproperty in a > SQL context, it is meant to have real meaning in that context, and in > all cases it should be an InstrumentedAttribute. Even an end-user- > defined attribute will use comparable_property() and will ensure > that it generates reasonable SQL.
The specific problem I'm referring to is more that, in effect, the first version digs around in the mappers of the entities in the query and tries to find the key of some property corresponding to the expression. Then, during the actual merge, it does getattr(instance, key_name) (where instance is some instance of the user's mapped class in the results) to get a value to compare on. If what comes out of that attribute is too dissimilar from what actually in the database (due to some custom getter function), the behavior will range from breakage (whatever it is doesn't implement __cmp__) to silently incorrect output (it orders differently from whats in the database). The second method avoids this entirely since it has direct access to the data in the same form the DB sees, via the hidden column entities. > But adding the clause explicitly > to the SQL is a good idea here regardless since its analgous to ORDER > BY, which also adds the clause to the SQL (just in a different way). Yeah, I had wondered to myself if it would be possible to use clauses added by ORDER BY to do the merge, but I wasn't sure if it always added a clause to the selection, since some DBs don't require that (for example PostgreSQL, which can ORDER BY arbitrary expressions that don't themselves appear in the selected columns). I haven't bothered to check if the various dialects exploit such features, if so it would make such reuse more difficult than it probably already would be. It's just a blue sky idea anyway. > I haven't looked at the code yet....but you should also take a look > at the Query implementation in user_defined_state (which is going to > be 0.5 RSN) to see if you need to make any modifications over there. > in particular the implementation style of a "generative" method is > different and you might want to use that newer system. > > It might be worth it for us just to totally consider this feature > within 0.5 since 0.4 is quickly becoming feature-frozen. That sounds good, actually the _generative decorator simplifies things for me somewhat. I'm in the process now of rebasing against user_defined_state. The one thing I haven't checked yet is if inheritance breaks the meth = _generative(__foo_condition)(meth) call, since it's not clear to me that the __foo_condition names are bound inside ShardedQuery's definition. I haven't gotten far enough that I can test it. Could be interesting :) Kyle --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "sqlalchemy" group. To post to this group, send email to sqlalchemy@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en -~----------~----~----~----~------~----~------~--~---
