On 01/25/2016 01:41 PM, Lele Gaifax wrote: > Mike Bayer <mike...@zzzcomputing.com> writes: > >> throwing an isinstance() right into the otherwise no-latency __getitem__ >> (the pure python version) there is making me think this may be better as a >> key fallback, so the expense is limited just to the negative integer case >> (or another idea, the negative int keys could be pre-populated the same way >> the positive ones are). what do you think? > > This is exactly what made me wonder about the right location of the "adaption" > logic. > > I will rewrite the tweak, but can you clear your indication: is it something > like > > > def __getitem__(self, key): > try: > processor, obj, index = self._keymap[key] > except KeyError: > if isinstance(key, util.int_types) and key < 0: > positive_key = key + len(self) > try: > processor, obj, index = self._keymap[positive_key] > except KeyError: > processor, obj, index = self._parent._key_fallback(key) > else: > processor, obj, index = self._parent._key_fallback(key)
so in this case I just don't like how there are different ways of looking up a key and they are spread out, sometimes based on if the key exists already, sometimes based on int + negative. If we hit KeyError here, that we have just one direction: "do the key fallback" makes it easier to understand and more portable. > > or instead about moving the logic enterely within the _key_fallback() method, > leaving the __getitem__() one exactly as it is/was? The former is somewhat > faster, at the cost of a tiny code dup, because there isn't any extra lookup > and function call, while the latter has the advantage that the C would remain > untouched (but I'd make a different commit removing the redundant index > computation under Py2). If you hit and catch a KeyError, that's already a big performance hit, so if we then call upon _key_fallback() and that's where we look at the negative integer, I wouldn't think that's too big of a deal. Let's consider going with the third option I mentioned, I dont think this will add too much expense if you could test this out please; the ResultMetaData has been highly reworked for 1.1 in any case: diff --git a/lib/sqlalchemy/engine/result.py b/lib/sqlalchemy/engine/result.py index cc4ac74..7e84463 100644 --- a/lib/sqlalchemy/engine/result.py +++ b/lib/sqlalchemy/engine/result.py @@ -214,6 +214,9 @@ class ResultMetaData(object): self._keymap = dict([ (elem[0], (elem[3], elem[4], elem[0])) for elem in raw + ] + [ + (elem[0] - num_ctx_cols, (elem[3], elem[4], elem[0])) + for elem in raw ]) # processors in key order for certain per-row > > Thank you, > ciao, lele. > -- You received this message because you are subscribed to the Google Groups "sqlalchemy" group. To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+unsubscr...@googlegroups.com. To post to this group, send email to sqlalchemy@googlegroups.com. Visit this group at https://groups.google.com/group/sqlalchemy. For more options, visit https://groups.google.com/d/optout.