Add function name and username as a comment in all SQL queries

This will save me a lot of greps to find the offending queries when doing tuning.

Here is a patch that seems to work.

Don't see why this should not be in Drupal 6. It is non-intrusive at all.

That's not the only place there are queries.

See http://drupal.org/node/168812. In fact, if we could put this in that logic there, it might be nice, so we don't have to do backtraces in two+ places.

@moshe:

i don't fully understand your comment. we issue queries that don't run through db_query()?

Yes. Several.

From http://drupal.org/files/issues/db_backtrace_26.patch:

$query_functions = array('db_query', 'pager_query', 'db_query_range', 'db_query_temporary', 'update_sql');

You're right that we can't consolidate it though.. the code there only executes on error condition. But you can probably swipe the logic to make sure it prints the right function.

@moshe

I see no harm in having it always there. If you have a large live site that is creaking under load, and no devel, you are out of luck.

The only thing I am not sure of is whether David Strauss' master/slave thing parses the query to determine if it is a read or write one. If it does, then the /* ... */ part has to be stripped out or ignored.

@webchick

You are right about this being scattered over all those functions. Should we go and add these two lines to each of these functions?

This version addresses moshe's two comments:

- Makes this conditional on the dev_query variable being set.

- Adds the user name before the function name, this includes anonymous too (why not ...)

Need opinions from webchick and others on how best to handle the other functions. We can make these few lines a centralized function and call it from all the other 4 functions.

Here it is. Seems to work fine for me.

More testing needed.

Plus:

- Good that we only run this when devel is on.
- SQL rewrites are thankfully already done by this stage, so we don't need to be afraid of these comments messing up the rewrites.

Minus:

- There are some code style problems with concatenation.

Code style fixed.

Should be ready to go.

Doh, although I just committed this patch, it occurred to me that this opens an attack vector with usernames. If I have "Gábor */ DROP TABLE system;" as my username, and the site has devel module turned on for whatever reason, I can drop the table. So let's close this attack option.

Hm, I also looked into the user name validation code, and it seems these chars are not allowed, so we are safe there. So tasks:

- let's document this in the mysql and mysqli _db_query()
- why was the pgsql code left out of the fun?? pgsql does support /* */ types of comments

This patch documents the changes to the two files (mysql, and mysqli).

I don't use PostgreSQL, so someone who does can add that.

We should not rely on the username validation code; I'm sure their are various ways to create users (contrib?, imports?) that may not run the validation.

- If you only need t() for the anonymous name, then this is not a real problem IMHO. Feel free to leave that out, but fix the comment to suggest that it was actually removed from around Anonymous. Now the comment hangs in the air.
- Also to be sure that values are properly sanitized, I would put the str replace around the ternary operator, not only on the username, but also on the anonymous name. There is nothing stopping admins from screwing up all their SQL queries with a fun anonymous user name otherwise.

Thanks, committed.