Conversation
The current MySQL query generation logic does not quote the primary keys in generated subqueries, which can result in queries like:
```mysql
DELETE FROM `datablock_storage_kvps` WHERE (datablock_storage_kvps.project_id, datablock_storage_kvps.key) IN (SELECT project_id,key FROM (SELECT DISTINCT `datablock_storage_kvps`.`project_id`, `datablock_storage_kvps`.`key` FROM `datablock_storage_kvps` WHERE `datablock_storage_kvps`.`project_id` = 13) __active_record_temp)
```
Which can, depending on the names given to the keys, result in errors like:
ActiveRecord::StatementInvalid: Mysql2::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'key FROM (SELECT DISTINCT `datablock_storage_kvps`.`project_id`, `datablock_stor' at line 1
This PR adds logic to quote the parameters in both the `WHERE` and `SELECT` clauses, resulting in a query like:
```mysql
DELETE FROM `datablock_storage_kvps` WHERE (`datablock_storage_kvps`.`project_id`, `datablock_storage_kvps`.`key`) IN (SELECT `project_id`,`key` FROM (SELECT DISTINCT `datablock_storage_kvps`.`project_id`, `datablock_storage_kvps`.`key` FROM `datablock_storage_kvps` WHERE `datablock_storage_kvps`.`project_id` = 13) __active_record_temp)
```
|
Ok, well tests pass, so good by me. Let me know once you take this out of draft status. |
|
Great, thanks! I was thinking of adding some new tests to cover this case, but now I'm thinking that might be more trouble than its worth, particularly given how situationally specific it is. Happy to do so if you think it'd be worthwhile, but otherwise I'd say this is ready for review! :) |
|
A test is always good! |
|
Would you say a test for this is worth adding yet another text fixture? I made a couple of attempts to create a test case which reproduces the original error by modifying existing fixtures, but kept breaking other tests in the process 🙃 I can easily create a new fixture just for this test, but I don't want to add unnecessarily to the maintenance burden here. What's your preference? |
|
Good question - I was trying to avoid adding new fixtures. But now that CPK has been superseded by what is built into ActiveRecord, I leave it up to you. |
2 participants
The current MySQL query generation logic does not quote the primary keys in generated subqueries, which can result in queries like:
Which can, depending on the names given to the keys (in this case, "key"), result in errors like:
This PR adds logic to quote the parameters in both the
WHEREandSELECTclauses, resulting in a query like: