metabase / toucan

A classy high-level Clojure library for defining application models and retrieving them from a DB
Eclipse Public License 1.0
570 stars 49 forks source link

Setting :identifiers for JDBC connection is broken with toucan.db/query in 1.13.0 #60

Closed miikka closed 5 years ago

miikka commented 5 years ago

We're setting :identifiers for our JDBC connection to a function that converts underscores to dashes. However, Toucan 1.13.0 introduced the this change to toucan.db/query which overrides our identifiers function.

Now our underscores do not get transformed to dashes, which of course breaks many things, and there's no workaround. I think that the best thing would be to not set :identifiers here and just let clojure.java.jdbc handle it once the version with the fix has been released.

@camsaul

camsaul commented 5 years ago

@miikka would it fix things for you if I changed

(into options {:identifiers u/lower-case}

to

(merge {:identifiers u/lower-case} options)

That way if options specifies :identifiers it will override the default we added

miikka commented 5 years ago

The trouble is that we set :identifiers already in the JDBC connection map, i.e. what is returned by (connection). This way we do not need to pass it to every db/query invocation in the codebase. What you propose would still break our code, because the third parameter to jdbc/query would still be {:identifiers u/lower-case} be default. But like you say, at least it does allow the workaround by passing in options every time.