crate / sqlalchemy-cratedb

SQLAlchemy dialect for CrateDB.
https://cratedb.com/docs/sqlalchemy-cratedb/
Apache License 2.0
3 stars 2 forks source link

Support: Generalize one more use case into `support.util.refresh_table` #140

Closed amotl closed 4 months ago

amotl commented 4 months ago

Suggestion

@seut suggested another improvement at https://github.com/crate/sqlalchemy-cratedb/pull/28#pullrequestreview-2135990302:

I think I'd prefer moving this quoting logic into the refresh_tablefunction, by change it's signature to consume an optional schema. This would avoid that it will be called with unquoted or invalid relation names.

Status

This patch refactors the auxiliary logic into support.util.refresh_table, but doesn't do anything else yet, in order not to complicate the interface too quickly/early.

Q&A

Maybe I am currently not seeing the spot for important improvements, which should be done right from the start: So, please advise if you still would like to introduce an optional schema name as function argument, and also please suggest further improvements you would like to see in this area.