Skip to content
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

[infra] Improve quote_table_name to accept full-qualified table identifiers #168

Closed
amotl opened this issue Jun 13, 2024 · 1 comment · Fixed by #172
Closed

[infra] Improve quote_table_name to accept full-qualified table identifiers #168

amotl opened this issue Jun 13, 2024 · 1 comment · Fixed by #172

Comments

@amotl
Copy link
Member

amotl commented Jun 13, 2024

@seut: Do you think this routine needs to be improved? You mentioned something about "quoting going south". Maybe the root cause is here, because the routine may only handle a few situations correctly?

Yes the issue here is that it will not quote anything if the ident contains a . like foo.bar. This looks a bit wrong as it normally should quote an ident which contains dots, this would be a valid table name for PG but not for CrateDB which forbids using a . inside a table identifier (see https://cratedb.com/docs/crate/reference/en/latest/general/ddl/create-table.html#naming-restrictions).
But then I wonder why you're not using sqlalchemy.sql.expression.quoted_name which works similar afaik.
That it tries to detect a FQ identifier, splits it and quotes all parts separately, would be a bit custom but needed if the schema and table idents are not quoted dedicated.

Originally posted by @seut in #88 (comment)

@amotl
Copy link
Member Author

amotl commented Jun 13, 2024

@seut submitted a patch including a corresponding improvement. Thank you.

@seut seut closed this as completed in #172 Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant