Re: [sqlalchemy] Baked queries vs before_execute hook

[Martijn van Oosterhout]

On Thursday, October 12, 2017 at 5:47:53 PM UTC+2, Mike Bayer wrote:
>
>
> It sounds like you are getting back inconsistent SQL for the same 
> query based on some external context that is not being considered as 
> part of the cache key.  This would indicate that you are probably 
> modifying the select() object *in place* inside your before_execute 
> hook.If your before_execute() hook returns a *new* select() 
> object, it would not pollute the cache with your late-modified value 
> against the cache keys. 
>
> That is, it's the difference between calling 
> select.append_whereclause() and select.where().The 
> before_execute() hook would need to be set up with retval=True and 
> return the new statement and parameters. 
>
>
Bingo! Looking at the code it has append_from() and append_whereclause() 
calls, so it's probably modifying in place. Sigh. That probably means this 
is going to break the caching in even more spectacular ways which we 
haven't yet spotted. The action of the hook is indeed dependant on 
something that is not part of the query, namely the "perms" field which 
only exists in our own CheckedQuery class.

The concept of the hook is pretty simple. It looks through the query for 
which tables it uses and if it finds a table marked as "special" it adds a 
filter and possibly some joins. I'm fairly sure this could be done safely 
using the Visitor pattern, in practice it's one big ball of spaghetti 
no-one wants to touch. Essentially it looks for a table and replaces it 
with a subquery, but it works by looping/recursing through the fields of 
the query itself. Ugh.

I think we're going to have to drop the idea of the hook in the long term, 
and at least short-circuit it for baked-queries, putting the query 
rewriting in our wrapper, then it can be cached like the rest. Especially 
since in 1.2 lazy loading is going to trigger this (though probably disable 
lazy loading in most places).

Caching the query rewriting isn't a bad plan either. But it looks like our 
query rewriting is more of a liability than I thought.

Thanks for the help!

Have a nice day,
-- 
Martijn

-- 
SQLAlchemy - 
The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable 
Example.  See  http://stackoverflow.com/help/mcve for a full description.
--- 
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.

Re: [sqlalchemy] Baked queries vs before_execute hook

[Mike Bayer]

On Thu, Oct 12, 2017 at 10:54 AM, Martijn van Oosterhout
 wrote:
> Hi,
>
> Recently we've been looking into the baked query feature as a method of
> speeding up query compilation. We also use a before_execute hook to modify
> the query before execution to handle permission related stuff. One thing
> that turned up was that when using a baked query that it cached the final
> string version.
>
> What this means is that the baked query framework caches the results of the
> before_execute hook meaning that queries occasionally produce the wrong
> output in situations where the before_execute hook would do something
> different. I'm not clear if this is a bug or a "you break it you get to keep
> both pieces".
>
> We worked around this (yes, before_execute hooks are evil) but this became
> more urgent when an old product accidentally got SQLAlchemy 1.2.0b where
> baked queries are used for lazy loading, which caused all sorts of funky
> errors. Whoops!

So there are two hooks where the pre-compiled SQL can be modified such
that it will still get cached, there's the Query level
"before_compile" hook, and the Engine-level "before_execute" hook.
Both of these operate before the SQL string is generated, which
ultimately is cached based on the identity of the Core select() object
itself.

It sounds like you are getting back inconsistent SQL for the same
query based on some external context that is not being considered as
part of the cache key.  This would indicate that you are probably
modifying the select() object *in place* inside your before_execute
hook.If your before_execute() hook returns a *new* select()
object, it would not pollute the cache with your late-modified value
against the cache keys.

That is, it's the difference between calling
select.append_whereclause() and select.where().The
before_execute() hook would need to be set up with retval=True and
return the new statement and parameters.

This would of course defeat part of the caching, unless you could
organize your before_execute() hook such that the *same* select()
object is returned each time given the same input select().  That is,
you might want to build your own local "cache of our modified
select()" objects so that the caching of generated SQL still takes
place, if that makes sense.If not, provide a short runnable
example of how your before_execute() hook works and I can attempt to
demonstrate.





>
> I'm wondering if there is a way of at least detecting this? Such that if a
> before_execute hook changes a query that the result is automatically not
> cached. That would at least prevent things from breaking unexpectedly. But
> long term, caching the compilation is really nice and so we'd like to be
> able to keep that feature. Our hook is predictable such that with the same
> input query and a parameters which is stored in the Query object you always
> get the same result. So it would in theory be possible to work with the
> baked query framework, but I'm totally not clear how that would work.
>
> Any ideas?
>
> As an aside, we worked around a few things by creating a WrappedBakedQuery
> class, which allowed us to do thing like:
>
> baked_query += lambda q: q.filter(Table.col == bind_param('foo'))
> baked_query.set_param('foo', 1)
>
> Which worked better in our setup.
>
> Have a nice day,
> --
> Maritjn
>
> --
> SQLAlchemy -
> The Python SQL Toolkit and Object Relational Mapper
>
> http://www.sqlalchemy.org/
>
> To post example code, please provide an MCVE: Minimal, Complete, and
> Verifiable Example. See http://stackoverflow.com/help/mcve for a full
> description.
> ---
> 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.

-- 
SQLAlchemy - 
The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable 
Example.  See  http://stackoverflow.com/help/mcve for a full description.
--- 
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.

[sqlalchemy] Baked queries vs before_execute hook

[Martijn van Oosterhout]

Hi,

Recently we've been looking into the baked query feature as a method of 
speeding up query compilation. We also use a before_execute hook to modify 
the query before execution to handle permission related stuff. One thing 
that turned up was that when using a baked query that it cached the final 
string version.

What this means is that the baked query framework caches the results of the 
before_execute hook meaning that queries occasionally produce the wrong 
output in situations where the before_execute hook would do something 
different. I'm not clear if this is a bug or a "you break it you get to 
keep both pieces".

We worked around this (yes, before_execute hooks are evil) but this became 
more urgent when an old product accidentally got SQLAlchemy 1.2.0b where 
baked queries are used for lazy loading, which caused all sorts of funky 
errors. Whoops!

I'm wondering if there is a way of at least detecting this? Such that if a 
before_execute hook changes a query that the result is automatically not 
cached. That would at least prevent things from breaking unexpectedly. But 
long term, caching the compilation is really nice and so we'd like to be 
able to keep that feature. Our hook is predictable such that with the same 
input query and a parameters which is stored in the Query object you always 
get the same result. So it would in theory be possible to work with the 
baked query framework, but I'm totally not clear how that would work.

Any ideas?

As an aside, we worked around a few things by creating a WrappedBakedQuery 
class, which allowed us to do thing like:

baked_query += lambda q: q.filter(Table.col == bind_param('foo'))
baked_query.set_param('foo', 1)

Which worked better in our setup.

Have a nice day,
-- 
Maritjn

-- 
SQLAlchemy - 
The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable 
Example.  See  http://stackoverflow.com/help/mcve for a full description.
--- 
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.