Closed (fixed)
Project:
Drupal driver for SQL Server and SQL Azure
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
28 Jan 2013 at 11:44 UTC
Updated:
15 Jun 2013 at 19:40 UTC
Jump to comment: Most recent file
Comments
Comment #1
xenphibian commentedThis is also true when a schema is creating a constraint, as shown here:
Note that the column name isn't quoted when creating any CHECK constraint.
This causes things like the private message module to fail to install because they have a constraint on the column 'public', but that could probably cause problems for any keyword that's used as a column name.
I did check that putting the column names for constraints between '[' and ']' fixes the problem as expected.
This also suggests that a test should be included in every database driver: Create a table that has columns with constraints, and defaults that has one column for each keyword and probably an index on each type. I can provide a simple example if that will help. This would need to be db driver specific (just like the list of keywords, database types, etc.).
Comment #2
Uncle_Code_Monkey commentedIf you look in
sqlsrv/database.inc, look for the constant definition lineconst RESERVED_REGEXP. You will see a long line defining all the reserved words that will get encapsulated in square brackets [] when executing the query. Just add what you need there.The dev edition of the driver already includes "public" so this patch puts "rule" in there for you. I did not place "hidden" in the list because it wasn't found as a reserve word in Microsoft's page that lists such things (but it's still trivial to add it if still required).
Comment #3
david_garcia commentedProposed solution: to massage db_query calls. Requires further testing.
Massage is already applied to queries issued via means other than db_query wich should be some sort of guarantee.
Sorry for the non-standard patch format.
Comment #4
david_garcia commentedPrevious patch messed up the driver's installation because it was not considering $bypassQueryPreprocess, and this is required during the driver's installation (install.inc) when defining user functions such as SUBSTRING and SUBSTRING_INDEX.
Summary of changes:
- Include "rule" in the regular expression of reserved words
- Override the "query" method so that queries are massaged. Consider $bypassQueryPreprocess in the override.
- Modifiy the condition used to apply the regular expression so that it will also admit queries that start with SELECT in LOWERCASE!
Comment #5
david_garcia commentedApply $quoteIdentifier when creating tables from SCHEMAS on field names when building CHECK constraints. Solves issue from second post, not original problem.
Comment #6
omegamonk commentedI applied '_sqlsrv-add_rule_rsv_word-1900840-2.patch' and 'schema.inc_.patch' to the dev release.
I didn't apply 'database.inc_.patch' for a few reasons. The addition of 'rule' to the list of reserved words was already added in '_sqlsrv-add_rule_rsv_word-1900840-2.patch'. The query method being added in this patch is redundant. PDOPrepare already performs preprocessing of each query, which is called by getStatement whenever a query is executed.
I am still taking a look at the case insensitive regex change in #4. There are a few regexes in the preprocessQuery function that I am going to evaluate and probably just update all at once. Since that isn't the core of this issue, I am marking this issue as fixed.
Comment #7
david_garcia commentedI've looked into this again and it's true that the original proposal of "massage db_query calls" was not correct, this is already done.
What actually fixed my problem was the case insensitive regex change! It would be good if at some point this change made it into the driver, i've come across several queries that start with "select" insted of "SELECT".