DynamoDB: Wrap all values in UPDATE statements in quotes #19
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 823 823
=========================================
Hits 823 823
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| """ | ||
| values_clause = self.deserialize_item(keys) | ||
|
|
||
| constraints: t.List[str] = [] | ||
| for key_name, key_value in values_clause.items(): | ||
| constraint = f"{self.DATA_COLUMN}['{key_name}'] = {key_value}" | ||
| constraint = f"{self.DATA_COLUMN}['{key_name}'] = '{key_value}'" |
There was a problem hiding this comment.
Right. Or we use some other notation, like $$. Even though that will not eliminate the need for escaping.
| """ | ||
| values_clause = self.deserialize_item(keys) | ||
|
|
||
| constraints: t.List[str] = [] | ||
| for key_name, key_value in values_clause.items(): | ||
| constraint = f"{self.DATA_COLUMN}['{key_name}'] = {key_value}" | ||
| constraint = f"{self.DATA_COLUMN}['{key_name}'] = '{key_value}'" |
There was a problem hiding this comment.
| constraint = f"{self.DATA_COLUMN}['{key_name}'] = '{key_value}'" | |
| key_value = str(key_value).replace("'", "''") | |
| constraint = f"{self.DATA_COLUMN}['{key_name}'] = '{key_value}'" |
There was a problem hiding this comment.
We can also check if there is a reusable method in sqlalchemy or crate-python, which would already have sufficient test coverage. And such a method would be easier to re-use, since we will need it several times.
There was a problem hiding this comment.
Hi. Thanks for tagging.
We are talking about quoting values on this spot, right? We provided a generic quoting function for schema- and table-names per DatabaseAdapter.quote_relation_name, based on SQLAlchemy's identifier preparer.
So, Nomen est omen, the identifier preparer is about quoting identifiers. I will have a look if there is something similar for values.
There was a problem hiding this comment.
On this spot, is it about proper quoting, or rather about escaping special characters within the value string at hand?
There was a problem hiding this comment.
I think that this SO conversation includes good and relevant content and pointers? Please correct me if I'm looking at the wrong thing.
There was a problem hiding this comment.
I will be merging as-is, including @hlcianfagna's suggestion. Generalizing and improving the ad hoc implementation, I am deferring to a subsequent iteration.
I actually didn't know this was possible. Is it documented somewhere / a secret ingredient just everyone knows, or even a general SQL feature available on all databases? |
amotl
force-pushed
the
main
branch
2 times, most recently
from
68ce5d8 to
31fe858
Compare
|
CI is failing because uploading to Codecov needs an authentication token, which is only available on this repository. I've verified the patch on my workstation, so I am confident it can be merged and released. Thank you very much. |
In the long term, we might need to look up the original data type and decide more selectively how to pass different data types. For now, let's pass everything as a string and leave it to CrateDB to auto-cast values.
This fixes a reported issue: