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.

Reply via email to