Merge lp:~wallyworld/launchpad/log-sql-bind-variables into lp:launchpad
| Status: | Merged |
|---|---|
| Approved by: | Robert Collins |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 11760 |
| Proposed branch: | lp:~wallyworld/launchpad/log-sql-bind-variables |
| Merge into: | lp:launchpad |
| Diff against target: |
44 lines (+10/-3) 1 file modified
lib/canonical/launchpad/webapp/adapter.py (+10/-3) |
| To merge this branch: | bzr merge lp:~wallyworld/launchpad/log-sql-bind-variables |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Robert Collins (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Add support for logging SQL bind values
Description of the change
Proposal
-----------
This branch provides an extra logging feature for tracing SQL execution. Currently, you can specify LP_DEBUG_SQL=1 to enable logging of the SQL that is executed, but the log does not include the bind parameters eg
select from foo where something = %s
Without the bind parameters being logged, it makes it harder to understand what is happening and to do things like detect duplicate SQL. eg for the above example, there could be 2 executions of that statement, either:
select from foo where something = 'bar'
select from foo where something = 'fred'
or
select from foo where something = 'bar'
select from foo where something = 'bar'
Clearly, the latter case represents something that could be optimised as part of performance tuning. But is it is difficult to see this without the bind parameters being logged.
Implementation
-------
Add support for a new environment variable: LP_SQL_DEBUG_VALUES
If this variable is true, and LP_SQL_DEBUG is also specified, then the SQL bind variables are logged.
NB the new setting has no effect if LP_SWL_DEBUG is not specified.
| Robert Collins (lifeless) | # |
| Robert Collins (lifeless) wrote | # |
Couple of small tweaks.
Looking at storm, you probably want to replace
param_strings = [
40 + (lambda x:
41 + x.get() if isinstance(x, Variable) else str(x))(p)
42 + for p in params]
with
param_strings = Connection.
which will change get() to get(to_db=True) as well as being more pithy.
This may fix your test issue, though there is an obvious open question of 'why is it failing' - and the problem is - I don't know (other than that there are apparently test failures in trunk (eep)).