Fork me on GitHub
#clojure-dev
<
2019-07-25
>
seancorfield00:07:47

I'm asking this here because I suspect the answer may depend on knowledge of internals of the compiler and/or the JVM. Background: next.jdbc relies heavily on protocols and uses "native" Java types where possible for performance. That makes it really hard to pass around options since you can't "attach" them to arbitrary Java objects. There's a common request from users to be able to globally set a default for :builder-fn -- which determines the naming conventions used when turning ResultSet objects into Clojure hash maps. Currently, the code in next.jdbc that handles that option does (get opts :builder-fn as-maps) in several places. All those places have a ResultSet object accessible so...

seancorfield00:07:49

...the suggestion has been made for next.jdbc to add a protocol, say DefaultBuilder, that has an implementation for Object that just returns as-map. So that would become (get opts :builder-fn (default-builder rs)) and the Object implementation of default-builder yields as-maps. Then users could extend-protocol DefaultBuilder ResultSet (default-builder [_] my-custom-builder)...

seancorfield00:07:54

First off, what's folks' reaction to that sort of thing? It feels hacky to me but it also seems feasible and, I hope, wouldn't have much of a performance impact (which is one of my main concerns here).

seancorfield00:07:56

Second, given the constraints above, what would you suggest as a way to provide an overridable default for a function that is highly performant.

seancorfield00:07:32

I get the impression that Clojure itself still relies on a bunch of dynamic Vars for some of this -- but I'm not sure whether that's in any performance-sensitive code?

hiredman01:07:30

The guidance for extending protocols is something like "only do it if you own the protocol or the type", which I think is to promote interoperability. In this case the user doesn't own the protocol (next.jdbc does), and doesn't own the type (Java jdbc) does. Resultset is also, if I recall, an interface, which brings other considerations because there is nothing like prefer-method for protocols (but I'd don't entirely recall the details here), so it is harder to specify a different behavior for a particularly implementation of resultset

seancorfield01:07:14

Yeah, although SettableParameter and ReadableColumn essentially break that guidance already: the assumption is the user will extend them to arbitrary types that they want converted in/out of SQL-specific types (e.g., having java.time.Instant <--> java.sql.Timestamp)

seancorfield01:07:53

Those have default implementations only for nil and Object within next.jdbc (and mirror what clojure.java.jdbc did too).

seancorfield01:07:14

(hence users arguing there is precedent already 🙂 )

Alex Miller (Clojure team)01:07:21

I don't understand why (get opts :builder-fn as-maps) is not sufficient?

seancorfield03:07:54

@alexmiller Because as-maps is not the default some people want -- and they don't want to override the default in every single call.

Alex Miller (Clojure team)05:07:56

hooking ResultSet with a protocol seems weird

seancorfield05:07:39

Yeah... That's why I figured I'd canvas folks here...

bronsa09:07:32

@alexmiller just thought I'd check with you first -- tools.reader ATM has quite an ugly hack in its implementation to support tagged-literal in clojure <1.7.0, am considering bumping minimum requirement from clojure 1.4 to 1.7 and getting rid of that

bronsa09:07:42

do you think that's reasonable?

andy.fingerhut18:07:25

Looks like perhaps some Google Groups changes went into effect recently. I received email about Google changing things there a month or so back, but didn't read the details of what would be changing. This may be unrelated, but there is a message waiting for moderation there that I currently do not have permission to approve or reject (or even look at), even though I got the usual email notification that a pending message is there.

Alex Miller (Clojure team)18:07:29

probably someone else moderated it before you got there

Alex Miller (Clojure team)18:07:10

hasn't been any change in the moderation or setup aspect

Alex Miller (Clojure team)18:07:30

they removed a bunch of features and simplified the admin setup but it was mostly around things we don't use

andy.fingerhut18:07:03

Perhaps everything is normal. The thing that looked different was a kind of pop-up warning box saying "Permission to manage messages denied". Perhaps it has always done that when the moderation message set is empty, and I just haven't noticed it before.

Alex Miller (Clojure team)18:07:48

hmm, I haven't seen that. also, I don't know of anything that has changed

Alex Miller (Clojure team)18:07:21

are you logged into the normal google account?

Alex Miller (Clojure team)18:07:11

your acct is still listed as Manager role

andy.fingerhut18:07:45

Yes, logged out and back in to Google account to verify. Still see that message, although given there are no pending messages, the real test will be when there is a message to moderate.

andy.fingerhut19:07:59

Eventually Google groups will chip away one capability of each Manager role person, until it takes 4 of them to do everything 🙂 (referring to this for me, and messages you send there showing up in the Groups page, but not in people's inboxes who subscribed)

Alex Miller (Clojure team)19:07:21

almost everything about google groups is worse than it was 5 years ago

Alex Miller (Clojure team)20:07:40

well I guess you shouldn't use that then :)

Alex Miller (Clojure team)20:07:56

seriously though... will take a look

seancorfield22:07:55

Looks like the non-SSL redirects to the SSL version now. Thank you!

seancorfield22:07:55

Looks like the non-SSL redirects to the SSL version now. Thank you!