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

Working with Postgres Jsonb #58

Open rlander opened 5 years ago

rlander commented 5 years ago

I'm trying to integrate Toucan into a project that uses Postgres, but I can't insert into a Jsonb column. Here's the model:

(m/add-type! :json
  :in json/generate-string
  :out #(json/parse-string % keyword)) 

(m/defmodel Integration :integrations
  m/IModel
  (m/types [_]
    {:data :json}))  

And here's the error:

user=> (db/insert! Integration {:registration "123" :data {"a" "b"}})
(db/insert! Integration {:registration "123" :data {"a" "b"}})
2019-04-09 19:04:13,218 [nRepl-session-f8c664bb-fa71-4a5c-b061-be9ef18ff331] DEBUG toucan.db - DB Call: INSERT INTO "integrations" ("registration", "data") VALUES (?, ()) 
Execution error (PSQLException) at org.postgresql.core.v3.QueryExecutorImpl/receiveErrorResponse (QueryExecutorImpl.java:2477).
ERROR: syntax error at or near ")"
  Position: 66

I've followed the docs to the letter, but it doesn't work. Thanks!

@camsaul

rlander commented 5 years ago

Answering my own question, it seems that the problem is in the (m/types) function that isn't working. Instead, you should use (m/merge). Here's a working implementation in case anyone finds it useful:

(defn- clj->pgobject
    [value]
    (doto (PGobject.)
      (.setType "json")
      (.setValue (json/generate-string value))))

(m/add-type! :json
  :in clj->pgobject
  :out #(json/parse-string (.getValue %) keyword))

(m/defmodel Integration :integrations)

(extend (class Integration)
  m/IModel
  (merge m/IModelDefaults
    {:types (constantly {:data :json})}))
camsaul commented 5 years ago

Thanks for posting your solution @rlander.

I think in the future I want to move to a more idiomatically-Clojure way of doing things, using multimethods instead of the current protocols + non-Clojurey merge m/IModelDefaults, e.g. something like

(m/defmodel Integration :integrations)

;; Integration.data should be treated as JSON
(m/defmethod m/types [Integration :data] [_ _]
  :json)

This would have the benefit of letting you for example define a type for all columns with the same name, e.g.

;; All `data` columns should be treated as JSON
(m/defmethod m/types [Object :data] [_ _]
  :json)

And ideally allowing you to put the models in a hierarchy of some sort so you could do something like

(m/defmodel ModelWithJSONData :abstract? true)

(m/derive Integration ModelWithJSONData)

;; all models that derive from ModelWithJSONData should have their :data column treated as JSON
(m/defmethod m/types [ModelWithJSONData :data] [_ _]
  :json)

Doing this in a way that is minimally disruptive to existing projects is my biggest concern so I'm letting these ideas marinate for a while

rlander commented 5 years ago

Thanks for the response! I agree that multi-methods would be easier to easier (and more idiomatic) to extend than the current way, though I'm not sure I like the hierarchy part. I don't see a way around releasing a breaking version though. Anyways, I'd be up to contribute if you don't mind. Should I open a new issue and close this one?

miikka commented 5 years ago

I've had some really weird problems with PGobjects caching something when I use tools.namespace.repl to reload code. My colleagues think this is due to a bug deep in the Postgres JDBC driver 🤷‍♂ . As a workaround, I CAST the JSON string to JSONB:

(require '[honeysql.core :as sql])

(defn ->jsonb [data]
  (sql/call :cast (json/generate-string data) :jsonb))

(m/add-type! :jsonb
  :in ->jsonb
  :out #(json/parse-string % keyword))

FWIW, I use m/types like the docs say and like you show in the opening message, and I haven't had any problems.

vldmr-k commented 1 year ago

is it possible to search by jsonb field?