Pass the connection to query transformers #47220
Conversation
In the case the comment existed we would be calling this method twice before.
| @@ -1113,7 +1113,7 @@ def log(sql, name = "SQL", binds = [], type_casted_binds = [], statement_name = | |||
|
|
|||
| def transform_query(sql) | |||
| ActiveRecord.query_transformers.each do |transformer| | |||
| sql = transformer.call(sql) | |||
| sql = transformer.call(sql, self) | |||
There was a problem hiding this comment.
I don't think those transformers were intended to be public API, so I'm not bothering with allowing transformers that accept only one parameter. But let me know if I got it work @byroot about the intention of this being public.
There was a problem hiding this comment.
Yes, not intended to be public, at least not for now. I'd see value is such API being public, but I'd wouldn't do it without putting some thoughts into it first.
| attr_accessor :schema_cache | ||
|
|
||
| def connection_class; end | ||
| def checkin(_); end | ||
| def remove(_); end | ||
| def async_executor; end | ||
| def db_config |
There was a problem hiding this comment.
This change sucks. It is only required because when checkind out a connection form the pool, we first create the object, check if the database version satisfies the requirements, and then we assign the pool to the connection.
When we are checking the database version, we are doing a query, so we try to annotate the query with the tags, but the pool isn't assigned yet, so pool.db_config would return an exception.
This is because the NullPool doesn't satisfy the ConnectionPool interface, which we should fix.
As the only queries that happen without a pool are very likely internal queries I'm retuning an object that respond to anything, without caring if that object have the same interface of the real configs.
rafaelfranca
force-pushed
the
rm-connection-to-tag-logs
branch
from
259cd23
to
de9d7f6
Compare
This will avoid that this test mutates the global state of ActiveRecord::QueryLogs.taggings and thus affect other tests.
Sometimes we need to tag the queries with information that are in the database connection that is making the query, so let's add it to the context.
Previous this tag would get the information only from ActiveRecord::Base connection, which in a multi-database application would be wrong. This get the information from the connection that is going to be used to execute the query. We also had to change the NullPool to make this work. It is only required because when checking out a connection form the pool, we first create the object, check if the database version satisfies the requirements, and then we assign the pool to the connection. When we are checking the database version, we are doing a query, so we try to annotate the query with the tags, but the pool isn't assigned yet, so `pool.db_config` would return an exception. This is because the NullPool doesn't satisfy the ConnectionPool interface, which we should fix. As the only queries that happen without a pool are very likely internal queries we are retuning an object that respond to anything, without caring if that object have the same interface of the real configs.
rafaelfranca
force-pushed
the
rm-connection-to-tag-logs
branch
from
de9d7f6
to
892f326
Compare
Sometimes we need to tag the queries with information that are in the database connection that is making the query.
Exposing the connection in the
contextparameter for the tagging procs we can read the information we need.This also allow us to fix the default tagging for database information that were always using the
ActiveRecord::Baseconnection, to use the right connection.