search

korma / Korma

Tasty SQL for Clojure.
http://sqlkorma.com
1.48k stars 222 forks source link

with-db doesn't honor db-specifc naming #282

Open immoh opened 9 years ago

immoh commented 9 years ago

From @ls4f in #280:

defdb sets the default naming, which is great but with-db doesn't honor what is set for the specific database. I figure it's not a huge issue since I use the same naming on all my databases, but it seems like a bug to me. If nothing else - it have no other reason to use defdb.

immoh commented 9 years ago

Fixed a24a69c2d904fb78bef1a56b3257502e320a5147

ls4f commented 9 years ago

It would appear my fix is not good enough (doesn't transform fields). Example : We can see db-spec has naming delenn.core=> (db-for :momchil) {:pool #<Delay@5927cbe9: :pending>, :options {:naming {:keys #<string$lower_case clojure.string$lower_case@5ea51d56>, :fields #<string$upper_case clojure.string$upper_case@4b473e40>}, :delimiters ["\"" "\""], :alias-delimiter " AS ", :subprotocol "firebirdsql"}} And it works for exec-raw (which is tested) delenn.core=> (korma.db/with-db (db-for :momchil) (korma.core/exec-raw "SELECT ID FROM ME" :results))

({:id 1} {:id 2} {:id 9})

(my db is upper case)

But not for a constructed statement delenn.core=> (korma.db/with-db (db-for :momchil) (korma.core/select "me" (korma.core/fields :id))) which gives

GDSException Dynamic SQL Error SQL error code = -204 Table unknown me At line 1, column 18 org.firebirdsql.gds.impl.wire.AbstractJavaGDSImpl.readStatusVector (AbstractJavaGDSImpl.java:2092)

I've been at it for about an hour but can`t really see what I'm missing and could use a hint so I can make another PR.

immoh commented 9 years ago

Looks like the options from the current db are not used by the sql engine. Not sure what would be the best way to fix it, maybe bound-options(https://github.com/korma/Korma/blob/master/src/korma/sql/engine.clj#L14) should be set in with-db.

ls4f commented 9 years ago

Tried getting it to use the options from korma.db/*current-db* but it started spitting out warnings. Should it contain anything special?

immoh commented 9 years ago

I have to say that I don't know. You can take a look what's in korma.confg/options or options set in the query map (that is bound to bound-options)

ls4f commented 9 years ago

Hmm ... guess I'll spend some more time on it.

immoh commented 9 years ago

Reopening as #284 and #293 were reverted.

immoh commented 8 years ago

@ls4f Do you know does this require #313 or other changes to work correctly or is it already fixed in master? At least we should add back tests that were reverted.

ls4f commented 8 years ago

Off the top of my head, I think removing korma.config solved this issue permanently. I remember the discussion on that mentioned certain cases when the dynamic vars are not bound and statement generation falls back to the default connection, so it might be a problem with delayed generation of anything - not sure.

rymndhng commented 7 years ago

Is there any update on this?