Fork me on GitHub
#honeysql
<
2021-02-14
>
seancorfield06:02:58

OK, seancorfield/honeysql {:mvn/version "2.0.0-alpha1"} is available for early testing... or [seancorfield/honeysql "2.0.0-alpha1"]... unfortunately http://cljdoc.org doesn't like this release so you can find the docs at https://github.com/seancorfield/honeysql/blob/v2/doc/getting-started.md (I'm working on fixing broken links etc so please let me know if you hit any problems).

catjam 3
kwrooijen09:02:02

Reading the docs now. I'm really happy with some of these changes (written in differences-fromt-1-x.md ) 😍

kwrooijen09:02:10

The removal of the reader literals is especially great

kwrooijen09:02:26

The new :cast keyword is actually going to fix a nasty bug for me

kwrooijen09:02:44

If you try to insert an empty array in Postgres without casting the type it'll actually fail. (Not sure why Postgres needs to know the type of the data if it's empty but ok). Now that I don't need to use sql/call this might actually be easier for me

kwrooijen09:02:57

It's going to be some work to upgrade to 2.0. Mainly because of this https://github.com/seancorfield/honeysql/issues/276 @seancorfield any chance you thought about this? Of if you'd want to implement something like this at all?

seancorfield18:02:42

@kevin.van.rooijen I'm open to suggestions while it's still in alpha.

seancorfield18:02:56

My initial reading of Gungnir's purpose is that it feels like something that should applied to SQL execution rather than SQL generation.

kwrooijen12:02:16

What do you mean by execution? Simple Gungnir usecase: Query:

{:select :* :from :user :where [:= :user/email "[email protected]"]}
Transformation:
string/lower-case :user/email
Result:
{:select :* :from :user :where [:= :user/email ""]}

seancorfield17:02:57

@kevin.van.rooijen I mean that the transformation "doesn't matter" until the value is about to be stored into the DB or has just been retrieved from the DB. You've attached Gungnir to SQL generation but a similar transformation could be done against next.jdbc around the setting of parameters and the reading of SQL values back from the DB.

seancorfield17:02:26

I'll take a deeper look at Gungnir over the next week or two and discuss my thoughts in more detail after that -- but my gut reaction is that attaching this to the set parameter / read result set functionality in next.jdbc would be "better" because then it would operate on things like update! and insert! from next.jdbc.sql.

kwrooijen19:02:35

I mean that the transformation "doesn't matter" until the value is about to be stored into the DB or has just been retrieved from the DB. I don't think this is correct. Because I'm lower-casing the email in my query, not the resulting values. This is so that I don't have any case matching issues

kwrooijen19:02:23

But maybe this can be solved in next.jdbc as you said

seancorfield19:02:07

You misunderstand: You need the parameter lowercased before you store it into the parameter, as part of the prepared statement setup, for your query. Which could happen in next.jdbc's set-parameter -- and which would then apply to all of the ways you could run queries, not just those that go through HoneySQL.

seancorfield19:02:47

And if this was wrapped around next.jdbc, Gungnir could also transform values coming back from the DB if needed -- although that is orthogonal.

seancorfield19:02:45

To illustrate my point: if you do (jdbc/execute! ds (-> (select :*) (from :user) (where [:= :user/email ""]))) Gungnir would intercept that but if you did (sql/find-by-keys ds :user {:user/email ""}) it wouldn't -- with Gungnir based on HoneySQL; but Gungnir could intercept the second version as well if it was based on next.jdbc. Does that make more sense?

kwrooijen20:02:22

Right, so basically what I'm doing now could be handled with set-params. I'll take a look at that this week then, sounds like a much better solution

seancorfield20:02:45

Feel free to ping me if you have Qs -- the qualified name is not present by the time set-parameter is called but the metadata, available through the PreparedStatement, may provide enough information.

seancorfield20:02:52

And although clojure.java.jdbc has a different protocol, the machinery should be the same, so you could have one ns for next.jdbc and a separate ns for c.j.j if you wanted to support both libraries.

kwrooijen20:02:25

I'm already using some features from next.jdbc so it's already required πŸ™‚

kwrooijen20:02:29

Thanks for the tip

seancorfield20:02:59

I just took another look at it and I don't think there's enough metadata to figure it out: the parameter metadata only has type information and no concept of naming; and the result set metadata is about what comes back not what goes in.

seancorfield20:02:03

Which means the level you need access to is above the SQL statement / parameters, unfortunately. 😞

kwrooijen21:02:45

Hmm that's unfortunate

kwrooijen21:02:24

Is it possible to patch this to add the extra metadata?

kwrooijen21:02:37

Would be a good excuse for me to dive into next.jdbc

seancorfield21:02:53

Yes, you could extend the protocol via metadata on a value-by-value basis -- the trick I've been using is to wrap the value in a function (`constantly`) so the protocol implementation can easily retrieve the original value.

seancorfield21:02:10

That's how the next.jdbc.types type adapters work.

seancorfield21:02:21

I've just been looking at the Gungnir source code: it looks like it overrides a specific set of operators within HoneySQL to provide the before-read transform of the values associated with certain (qualified) keywords -- is that accurate?

seancorfield21:02:21

(and it also wraps some of next.jdbc to achieve the same thing directly with insert! and update!?)

kwrooijen22:02:15

Right, with Gungnir you can transform specific qualified-keywords defined in the model https://kwrooijen.github.io/gungnir/model.html It supports three things: :before-save - Done within gungnir, separate from HSQL / NJ :before-read - Patched version of HSQL (linked in the issue of HSQL) :after-read - custom result-set builder with NJ https://github.com/kwrooijen/gungnir/blob/master/src/clj/gungnir/database/builder.clj

kwrooijen22:02:08

You can read me model document for the use cases / how they are implemented by the end-user. It's all based on qualified keywords

seancorfield22:02:22

Given HoneySQL v2's recursive descent formatting -- which is independent of the actual operators/function calls being used -- it's certainly a lot harder to implement a hook like that. I'll continue to give it some thought.

kwrooijen22:02:14

All right. I'm a bit in between projects right now so Gungnir doesn't have my highest priority at the moment. Worst case I'd have to walk the HSQL datastructure but hopefully we can prevent that

seancorfield22:02:05

What would you do with an expression like this in Gungnir? (where [:= :user/email [:|| "KEVIN" "@" ""]])

seancorfield22:02:32

(it would produce ["... WHERE email = CONCAT(?, ?, ?)" "KEVIN" "@" ""])

kwrooijen07:02:40

the [;|| "KEVIN" "@" ""] part would be in the user defined part. So you'd for example have this (end-user defined)

(defmethod gungnir.model/before-read :string/lower-case [_k v]
  (clojure.string/lower-case v))
And v woul be [;|| "KEVIN" "@" ""] . So the end-user can decide what to do with it

kwrooijen07:02:20

(In this example it would fail because we're using string/lower-case on a vector, but the user could change that)

seancorfield16:02:16

@kevin.van.rooijen Ah, good to know. OK, that gives me some ideas then...

kwrooijen16:02:53

Let me know if you need any more info (sorry for the occasional late replies, timezones πŸ˜„ )

seancorfield17:02:24

To be expected when I'm on Pacific time and you're on Dutch(?) time πŸ™‚

athomasoriginal21:02:54

Just upgraded my side project to V2 (10k loc all clojure)! Everything is working well and the upgrade went smoothly. Thanks for everything Sean and let me know if there is anything I can test!

seancorfield21:02:05

Oh, nice! Thank you!

seancorfield21:02:31

What database are you using @tkjone?

athomasoriginal21:02:42

Postgres πŸ˜‰

athomasoriginal21:02:51

Because of the upgrade I was able to β€’ simplify helper functions built around sql/call, β€’ remove nilenso β€’ gain clarity on my queries using postgres specific functions

seancorfield21:02:57

Do you use the nilenso lib?

πŸ‘ 3
athomasoriginal21:02:14

haha well, not anymore πŸ™‚

3
athomasoriginal21:02:47

The areas I can see future upgraders maybe having challenges is if, as you mentioned, there is use of nilenso but one forgot which keywords were nilenso specific. Not a big deal, I just totally forgot I was using exists [UPDATE]: The above issue is unrelated to the nilenso library. It was undocumented behaviour from HoneySQL V1. See the convo in the thread below

athomasoriginal21:02:08

Only other points of momentary confusion: β€’ The switch from hsql/call to :function-name - but you have that well documented, I for some reason spaced when reading it β€’ Switch from format/value to :lift - again, well documented, but I was looking for it in the V1 to V2 transition guide. Again, my assumption getting the best of me haha

seancorfield21:02:20

The only references to exists I see in the nilenso lib are as part of the if-exists / if-not-exists around create/drop table/extension?

athomasoriginal21:02:28

hmm. One sec. Let me look at the sql that was failing one more time.

athomasoriginal22:02:32

This is the line I was successfully using with nilenso

(->> {:exists
        {:select [:datname]
         :from   [:pg_catalog.pg_database]
         :where  [:= :datname dbname]}}
       (hsql/format)))
and when I run the above with hsql 2 I get this exception:
"Unknown SQL clauses: :exists"

athomasoriginal22:02:35

To get it to work I updated it to

{:select [[[:exists {:select ...}]]]}

seancorfield22:02:58

Hmm, interesting. I can't find any mention of that in nilenso's code.

seancorfield22:02:32

Ah, it's an undocumented part of HoneySQL 1.x!

athomasoriginal22:02:31

Shoot! My bad!! but that makes sense! After a while, I would have to read H1 source more and more to figure out how to do something.

seancorfield22:02:40

But I did migrate it -- it just needs to be added to the migration doc and probably elsewhere:

;; EXISTS should never have been implemented as SQL syntax: it's an operator!
  #_(is (= (format {:exists {:select [:a] :from [:foo]}})
           ["EXISTS (SELECT a FROM foo)"]))
  ;; select function call with an alias:
  (is (= (format {:select [[[:exists {:select [:a] :from [:foo]}] :x]]})
         ["SELECT EXISTS (SELECT a FROM foo) AS x"]))

seancorfield22:02:02

(that's from the v2 tests)

athomasoriginal22:02:40

Nice, so it won’t be allowed to be used as SQL syntax, correct?

seancorfield22:02:58

It never should have been allowed -- that was a bug in how it was added 😞

seancorfield22:02:32

It was added way back in 0.5.3

seancorfield21:02:43

Ah, I totally missed format/value -- I'll add that to the migration doc and the :lift special syntax. Thanks.

seancorfield22:02:38

(docs updated)

πŸ‘ 3