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

Support parameterized statements in SQL migrations #115

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dryewo
Copy link

@dryewo dryewo commented Apr 17, 2017

Currently, :up and :down migrations are vectors of strings. Each string is then wrapped in a vector and given to jdbc/execute! However, jdbc/execute! expects a vector in the format:

["SOME STUFF(?, ?)" "foo" 42]

? are placeholders, where the "foo" and 42 are substituted in a safe way.

Currently there is no possibility to make use of it when defining migrations in EDN.

Would be nice to be able to do something like:

{:up ["CREATE TABLE foo (id int)"
      ["INSERT INTO foo(id) VALUES(?)" 1]]
 :down ["DROP TABLE foo"]}

For example, I needed this when I was generating migrations dynamically, inserting some user-defined value into the table.

This patch enables the described functionality.

@weavejester
Copy link
Owner

Let me consider this. There might be a better way of achieving the same result.

Also, can you fix the commit message and make sure it's capitalised? So something like:

Add support for parameters to SQL migrations

Since SQL migrations are not necessarily loaded from edn files, I don't think that part of your commit message makes sense.

@dryewo dryewo force-pushed the vector-statements branch from 2d46c10 to dcd9fc6 Compare April 17, 2017 13:14
@dryewo dryewo changed the title Support parameterized statements in .edn migrations Support parameterized statements in migrations Apr 17, 2017
@dryewo dryewo changed the title Support parameterized statements in migrations Support parameterized statements in SQL migrations Apr 17, 2017
@dryewo
Copy link
Author

dryewo commented Apr 17, 2017

Done.

Another way might be using:

(if (sequential? s) s [s])

But I don't see any pros/cons.

@dryewo
Copy link
Author

dryewo commented Apr 17, 2017

Oh, actually, there is an issue about this already: #112

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants