-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bug: reserved words cause compilation error #48
Comments
The create table statement using sqlalchemy will succeed with reserved words, minus ansi reserved like "CURRET_DATE". The logic that needs to be fixed is where we build sql queries and dont let sqlalchemy auto quote reserved words. This happens in the merge statement and in the comparison logic to see if columns exist. Ideally we could pass a column name through sqlalchemy somehow to let it decide for us but if thats not possible we will need to keep a list of reserved words on the side and manually map them. In target-postgres Derek is taking the approach of using sqlalchemy as much as possible to avoid stuff like this caused by manually building queries, we can do that but it will be challenging with merge statements. Maybe its best to get the table metadata as the source for the columns to build the merge statement instead of the singer schema. |
Closes #48 Sqlalchemy handles some of the quoting for reserved words but we also build sql manually so those dont get handled. Also when doing column diffing like to decided whether a column exists or not we need to use uppercase column names because we created them as uppercase with quotes so case matters. I found a bug with the COPY logic when the stream has schema updates. Logged it in #53 and draft PR in #52. Once that is fixed we can add a schema change to the test records with no key properties so we assert alters work with reserved words when no key properties are being used.
I have a table that inserted some reserved words like
select
andupdate
from tap-meltanohub, on initial creation the merge statement fails because of a compilation error due to select/update not being quoted:Additionally on the second sync, where the table exists already with reserved words, it does a diff on the column names and incorrectly decides to create that column as if its missing. An error is thrown because it already exists. It looks like usually sqlalchemy returns all lowercase columns but in the reserved word case it comes back uppercase. In the DDL of the snowflake table its wrapped in double quotes so I think sqlalchemy is taking it literally while maybe lowercasing the rest of the columns, not sure.
Fixed in MeltanoLabs/target-postgres#35
The text was updated successfully, but these errors were encountered: