On 01/25/2016 01:41 PM, Lele Gaifax wrote:
> Mike Bayer <[email protected]> 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 [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sqlalchemy.
For more options, visit https://groups.google.com/d/optout.
