Function drupal_write_record() should enclose field names by backticks

Are you kidding me?

This doesn't need to be changed. $fields actually holds a bunch of different strings that need to be imploded. Based on the SQL syntax it's in which is summarized to be something like INSERT INTO <tablename> (<comma>, <separated>, <field>, <names>) VALUES (<various>, <printf>, <substitution>, <placeholders>).

I believe I'm correct so setting to won't fix.

I was just tripped up by this issue.

I posted to the support forum a thread here:
http://drupal.org/node/611102

At the very least, there ought to be some documentation around the use of db_query() and drupal_write_record() warning developers to check their field names against sql's reserved word list...

http://dev.mysql.com/doc/refman/5.0/en/reserved-words.html

I think we also just got burned by this issue- see #612176: field name in notifications_custom table causes SQL error. Drupal is clearly not adhering to the SQL standard here - no way should this be labeled "by design" or "won't fix".

subscribe

just wasted 30 mins just to discover that field names are not escaped.. *argh* .. had a field named "before" in my tables

Related issues are #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements and #315047: All schema API operations should be reserved-word safe.

See also the handbook pages List of SQL reserved words, and SQL coding conventions.

Since this seems to be for MySQL database type, maybe something like this would be suitable:

if ($GLOBALS['db_type'] == 'mysql' || $GLOBALS['db_type'] == 'mysqli') {
  $field_or_database_name = '`' . $field_or_database_name . '`';
}

It's the responsibility of the person defining the schema not to use reserved words. Refer to #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements for background and (several years) of discussion.

Regarding @Dane Powell#6: there is no SQL standard for quoting database identifiers, as far as I know. Drupal is not doing anything particularly wrong here.

At the very least, documentation of the Schema API should caution developers to be aware that SQL reserved words can cause issues when used as table fields. Perhaps with links to some of the discussions referenced here.

Perhaps I should have been more precise and said MySQL standard? http://dev.mysql.com/doc/refman/5.0/en/identifiers.html

At any rate... I think we need to pick a direction and stick with it. Either (a) make developers responsible for not using reserved words, and publish a list of ALL of those words for every type of DB that Drupal is run on, or (b) properly escape field names for each database. Currently we only do half of (a), and it sounds like it's been like that for years.

Personally, (a) sounds like a lot more work and maintenance as new DB releases add new reserved words and possibly break existing modules. It should be the goal of Drupal to support as many architectures as possible, and yet for every new DB type supported existing modules will be broken and developers will have fewer and fewer valid words available to them. This goal and (a) thus seem to be in conflict to me.

I'd say b), as MySQL recommends usings backticks for escaping fieldnames (see link in #12), so why not go with it?
this issue causes pain for a lot of developers who aren't familiar with reserved words (and I bet they differ from dbms to dbms!)..

so whats the big deal of including two simple backticks for god's sake?

it would just be a small adjustment in db_prefix_tables() necessary..

@dsms: patches are always welcome. Fell free to roll one, especially if this is easy to fix.

I would recommend that you read the whole discussion in #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements first.

Bumping to D7, because bugs are fixed in the development version first, before being backported.

so whats the big deal of including two simple backticks for god's sake?

Because it's only two simple backticks on MySQL. It's 2 simple something else on other databases.

Not only do reserved words vary from one DB to another, the way one escapes field names varies from one DB to another. It is impossible to do that for static queries without regexing the heck of of them, which we are trying very hard to avoid ever doing. Even if we could automate escaping/quoting of field names for all built queries, that's going to be expensive with an extra method call for *every* field. Drupal is slow enough as is.

We're doing option A because option B is impossible.

I'm not saying that option B would be possible without a shift in how queries are handled. If I were designing things from the ground up I think I'd make sure that field names were wrapped (perhaps with []) just like table names are wrapped with {}, so we can operate on them easily if necessary. Actually, reading the other thread, just such a solution was proposed and benchmarks on it showed little or no performance hit. What if we took a hybrid approach, where developers can wrap field names if they choose to / need to, but aren't required to as long as they're sure that they aren't using reserved words.

I know we are re-arguing what's already been argued in the other thread, but I feel like the issue was never really resolved there - I didn't see a convincing argument as to why allowing field names to be wrapped wouldn't be viable.

Since this is now a question about the DB API specifically and how it deals with reserved words, I'm marking this as a duplicate of #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements. If someone has a *new* idea, we can revisit that in Drupal 8. For Drupal 7, reserved words are documented in the handbook. Just don't use them and you'll be fine. If you find a module that is, that's a bug, please file a patch.