If field name is a keyword, the query fail. Can this be solved?
Can you add [] around the field name in select queries?

Elysia Cron has a query with 'rule' as field name but this is a keyword and gives an error with ms sql server.

Comments

xenphibian’s picture

This is also true when a schema is creating a constraint, as shown here:

PDOException: SQLSTATE[42000]: [Microsoft][SQL Server Native Client 11.0][SQL Server]Incorrect syntax near the keyword 'public'.: CREATE TABLE [{pm_tags}] ( [tag_id] int NOT NULL IDENTITY CHECK (tag_id &gt= 0), [tag] nvarchar(255) NOT NULL, [public] smallint CONSTRAINT {pm_tags}_public_df DEFAULT 0 CHECK (public &gt= 0), [hidden] smallint CONSTRAINT {pm_tags}_hidden_df DEFAULT 0 CHECK (hidden &gt= 0), CONSTRAINT {pm_tags}_pkey PRIMARY KEY CLUSTERED ([tag_id]) ); Array ( ) in db_create_table() (line 2688 of ~\Drupal\includes\database\database.inc).

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.

CREATE TABLE [quotetest]
(
[tag_id] int NOT NULL IDENTITY CHECK ([tag_id] &gt= 0),
[tag] nvarchar(255) NOT NULL,
[public] smallint CONSTRAINT quotetest_public_df DEFAULT 0 CHECK ([public] &gt= 0),
[hidden] smallint CONSTRAINT quotetest_hidden_df DEFAULT 0 CHECK ([hidden] &gt= 0),

CONSTRAINT quotetest_pkey PRIMARY KEY CLUSTERED ([tag_id])
)

drop table [quotetest]

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.).

Uncle_Code_Monkey’s picture

Status: Active » Needs review
StatusFileSize
new1.35 KB

If you look in sqlsrv/database.inc, look for the constant definition line const 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).

david_garcia’s picture

StatusFileSize
new2.68 KB

Proposed 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.

david_garcia’s picture

StatusFileSize
new2.65 KB

Previous 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!

david_garcia’s picture

StatusFileSize
new251 bytes

Apply $quoteIdentifier when creating tables from SCHEMAS on field names when building CHECK constraints. Solves issue from second post, not original problem.

omegamonk’s picture

Assigned: Unassigned » omegamonk
Status: Needs review » Fixed

I 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.

david_garcia’s picture

I'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".

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.