weavejester / ragtime

Database-independent migration library
Eclipse Public License 1.0
612 stars 85 forks source link

Add option `:migrations-table-exists-sql` for SQL backed migrations #158

Closed mbezjak closed 3 weeks ago

mbezjak commented 1 year ago

Closes https://github.com/weavejester/ragtime/issues/157

mbezjak commented 1 year ago

@weavejester Here is my first pass to this. Does it look ok to you? Should I continue with adding the same option to ragtime.jdbc?

mbezjak commented 11 months ago

Implemented the same thing in jdbc project. The PR should be complete now.

@weavejester Let me know what you think. Especially about the comment above.

mbezjak commented 11 months ago

Updated as per comments. Thanks for looking into this. :smile:

Can you ensure all lines are 80 characters or less?

Done. Although, there are some prior lines not respecting it.

$ grep -R '.\{81\}' next-jdbc/src/ jdbc/src/
next-jdbc/src/ragtime/next_jdbc.clj:      ;; Returns unqualified maps as some databases (e.g. Oracle) can only return them
next-jdbc/src/ragtime/next_jdbc.clj:      (rs/datafiable-result-set conn {:builder-fn rs/as-unqualified-lower-maps})))
next-jdbc/src/ragtime/next_jdbc.clj:  ;; Remove db quote around the schema and table name; ex.: "\"schema\".\"table\""
mbezjak commented 1 month ago

Ping :smile:

weavejester commented 1 month ago

Apologies for the delay, this PR seems to have slipped through my inbox.

mbezjak commented 1 month ago

Done! Thanks for looking at this. :smile:

weavejester commented 3 weeks ago

Thanks! Can you change the commit message to:

Add :migrations-table-exists-sql option

Add a :migrations-table-exists-sql option in order to more efficiently
check for the Ragtime migrations table. Using database metadata lookups
via JDBC can be slow depending on the size of the database.

Fixes #157.

This is to ensure it adheres to the contributing guidelines. Once that's done I'll merge and cut a release. Thanks again for your patience.

mbezjak commented 3 weeks ago

Done! Thanks!