Merge lp:~zeitgeist/zeitgeist/explain-logs into lp:zeitgeist/0.1

[Mikkel Kamstrup Erlandsen]

Part 1) of our profiling quest can be merged now since it quite unintrusive and very useful in its own right

[lp:~zeitgeist/zeitgeist/explain-logs]

1580. By Mikkel Kamstrup Erlandsen on 2010-09-13

Add man page documentation for the ZEITGEIST_DEBUG_QUERY_PLANS envvar

[Markus Korn (thekorn)]

Looks good, and it does not have to be perfect as it is for internal use only, but I'd like to avoid the 'print' statement and put it in a logging statement. This way it would be possible to redirect logging output of the 'zeitgeist.sql' domain to some file etc.

Proposed diff:

=== modified file '_zeitgeist/engine/sql.py'
--- _zeitgeist/engine/sql.py 2010-09-13 20:55:00 +0000
+++ _zeitgeist/engine/sql.py 2010-09-14 07:08:20 +0000
@@ -38,9 +38,10 @@

 def explain_query(cursor, statement, arguments=()):
        log.debug("**********************************************")
- log.debug("QUERY:\n%s (%s)\nPLAN:" % (statement,arguments))
+ plan = ""
        for r in cursor.execute("EXPLAIN QUERY PLAN "+statement, arguments).fetchall():
- print r
+ plan += str(list(r)) + "\n"
+ log.debug("QUERY:\n%s (%s)\nPLAN:\n%s" % (statement,arguments, plan))
        log.debug("**********************************************")

 class UnicodeCursor(sqlite3.Cursor):

review: Approve

[Markus Korn (thekorn)]

I'm not sure, but maybe the queries are more readable if we replace '?' by the actual values:

=== modified file '_zeitgeist/engine/sql.py'
--- _zeitgeist/engine/sql.py 2010-09-13 20:55:00 +0000
+++ _zeitgeist/engine/sql.py 2010-09-14 07:40:46 +0000
@@ -38,9 +38,11 @@

 def explain_query(cursor, statement, arguments=()):
        log.debug("**********************************************")
- log.debug("QUERY:\n%s (%s)\nPLAN:" % (statement,arguments))
+ plan = ""
        for r in cursor.execute("EXPLAIN QUERY PLAN "+statement, arguments).fetchall():
- print r
+ plan += str(list(r)) + "\n"
+ statement = statement.replace("?", "%r") %tuple(arguments)
+ log.debug("QUERY:\n%s\nPLAN:\n%s" % (statement, plan))
        log.debug("**********************************************")

 class UnicodeCursor(sqlite3.Cursor):

But still, we can always adjust this in lp:zeitgeist itself, as it is for internal use only.

[Mikkel Kamstrup Erlandsen (kamstrup)]

I think it's valuable to be able to tell where we use ?-substitution, so I'll leave that as is. The print statement was a slip of mind, I'll fix that and then merge. Thanks for reviewing!

[lp:~zeitgeist/zeitgeist/explain-logs]

1581. By Mikkel Kamstrup Erlandsen on 2010-09-16

Don't use print for logging. Use the log already!

1582. By Mikkel Kamstrup Erlandsen on 2010-09-16

Prettify query plan logging a bit