Concise logging format for the binds of casted values #41136
Conversation
Related issue rails#41133. In rails#39106, I've allowed binds of casted values as an alternative of the legacy binds (an array of `[column || nil, value]`) to deprecate and remove the legacy binds support in a future version of Rails. At that time, I've respected the existing logging format for binds. i.e. ```ruby # legacy binds conn.select_all("SELECT * FROM events WHERE id IN (?, ?)", nil, [[nil, 1], [nil, 2]]) # (0.1ms) SELECT * FROM events WHERE id IN (?, ?) [[nil, 1], [nil, 2]] # casted binds conn.select_all("SELECT * FROM events WHERE id IN (?, ?)", nil, [1, 2]) # (0.1ms) SELECT * FROM events WHERE id IN (?, ?) [[nil, 1], [nil, 2]] ``` To improve the performance of generating IN clause, 72fd0ba avoids `build_bind_attribute` for each values, so now binds has become casted values. ```ruby conn.select_all(Event.where(id: [1, 2])) # (0.1ms) SELECT * FROM events WHERE id IN (?, ?) [[nil, 1], [nil, 2]] ``` Regardless of whether 72fd0ba avoids `build_bind_attribute` or not, the logging format for the binds of casted values is odd (at least not pretty to me). I'd like to concise the logging format to just use casted values. ```ruby conn.select_all("SELECT * FROM events WHERE id IN (?, ?)", nil, [1, 2]) # (0.1ms) SELECT * FROM events WHERE id IN (?, ?) [1, 2] conn.select_all(Event.where(id: [1, 2])) # (0.1ms) SELECT * FROM events WHERE id IN (?, ?) [1, 2] ```
kamipo
force-pushed
the
concise_logging_format_for_casted_binds
branch
from
65352ba
to
dd2cb97
Compare
|
I suspect that avoiding I've requested a review from the author to leave it up to decide which one to take. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Related issue #41133.
In #39106, I've allowed binds of casted values as an alternative of the
legacy binds (an array of
[column || nil, value]) to deprecate andremove the legacy binds support in a future version of Rails.
At that time, I've respected the existing logging format for binds.
i.e.
To improve the performance of generating IN clause, 72fd0ba avoids
build_bind_attributefor each values, so now binds has become castedvalues.
Regardless of whether 72fd0ba avoids
build_bind_attributeor not, thelogging format for the binds of casted values is odd (at least not
pretty to me).
I'd like to concise the logging format to just use casted values.