Fork me on GitHub
#sql
<
2019-04-26
>
seancorfield01:04:07

@ikitommi Thanks. Are you happy with that, given the constraints of being generic/general purpose I want to maintain for the library?

seancorfield01:04:31

Let me know if there are additional things I can do to cater for your use case -- without disadvantaging other users with more mainstream needs.

ikitommi10:04:44

@seancorfield I think it’s ok now. Few ideas for making the default case faster: 1) you could pass the whole sql-params in, gets rid of the slow first which effects the default case too, (`(nth sql-params 0)` would be faster for vectors anyway) 2) use non-qualified keys in options, like :sql-params here + introduce a Record that has all used options & supporting fast assoc etc. => client can create and populate Options record out of the execute cycle to go fast(er), not mandatory … my guess is that you could save few hundred nanos with those.

ikitommi10:04:51

also, added a abstraction for the sql-params in porsas: one can pass in either a plain String or a vector of string + params. In case there are no params, it saves as there is no creating or unpacking PersistentMaps.

seancorfield17:04:00

@ikitommi 1) I wondered about that when I wrote that code. Happy to "break" things by renaming :next.jdbc/sql-string to :next.jdbc/sql-params and passing the whole thing. 2) I really don't like the idea of a record with so many optional fields -- most folks use either no options or only a small handful. Optimizing for the empty case (well, just :next.jdbc/sql-params I guess) might be worthwhile but I'm not sure it's worth it really? And I picked a qualified name to signify that it's an internal-only option, so it's deliberately not in the same "namespace" as regular options. 3) I don't like Heisenparameters -- it was a mistake I made in clojure.java.jdbc and don't want to propagate into next.jdbc: having a parameter that is either a string or a vector starting with a string is just... ugh!

seancorfield17:04:23

So, yes, I'll do 1). If you can show that having a record optimization for :sql-params is really worthwhile, I'll consider 2) -- but not for "all" options (since I suspect record creation may slow down as the number of fields increases?).

seancorfield17:04:30

And, no, not 3) 🙂

seancorfield17:04:10

OK, created https://github.com/seancorfield/next-jdbc/issues/17 for 1) and implemented that change on master.

👍 4