java.jdbc

Should the default result-set-fn be vec instead of doall?

Details

  • Type: Enhancement Enhancement
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Declined
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

Rationale: obviously we want eagerness already, so the only difference here is what data structure we use. A vector has the advantage of being chunked when processed as a seq, which is probably the normal usage?

Happy to patch if there are no objections.

Activity

Hide
Gary Fredericks added a comment -

Sounds good, thanks.

Show
Gary Fredericks added a comment - Sounds good, thanks.
Hide
Sean Corfield added a comment -

If you can provide a benchmark that proves that - and a separate patch under a separate ticket - I'd look at that more favorably.

Show
Sean Corfield added a comment - If you can provide a benchmark that proves that - and a separate patch under a separate ticket - I'd look at that more favorably.
Hide
Gary Fredericks added a comment -

I don't want a vector in particular, I want a chunked seq so that subsequent processing is faster. If we go with just the chunked-seq approach (in result-set-seq) I think there's a very small chance of any noticeable behavior change.

My hunch is that most uses of java.jdbc would see some speedup as a result of chunking.

Show
Gary Fredericks added a comment - I don't want a vector in particular, I want a chunked seq so that subsequent processing is faster. If we go with just the chunked-seq approach (in result-set-seq) I think there's a very small chance of any noticeable behavior change. My hunch is that most uses of java.jdbc would see some speedup as a result of chunking.
Hide
Sean Corfield added a comment -

If you want a vec, specify it for :result-set-fn. At this point I don't want to change the default when there is existing production code depending on the old behavior.

Show
Sean Corfield added a comment - If you want a vec, specify it for :result-set-fn. At this point I don't want to change the default when there is existing production code depending on the old behavior.
Hide
Sean Corfield added a comment -

And it also occurred to me that if someone actually wants a vector result, they can just specify `:result-set-fn vec` directly. So it's easy to get the behavior you want without changing the behavior for all that existing code out there (I know, not much code relies on the new API yet, but I have it in production at World Singles so I don't want it to silently change under me at this point - we'd have to do a lot of testing to ensure it didn't adversely affect performance and/or stability).

Show
Sean Corfield added a comment - And it also occurred to me that if someone actually wants a vector result, they can just specify `:result-set-fn vec` directly. So it's easy to get the behavior you want without changing the behavior for all that existing code out there (I know, not much code relies on the new API yet, but I have it in production at World Singles so I don't want it to silently change under me at this point - we'd have to do a lot of testing to ensure it didn't adversely affect performance and/or stability).
Hide
Sean Corfield added a comment -

That sounds like over-complication to me. Is it solving a real problem that exists today with the `doall` approach?

Show
Sean Corfield added a comment - That sounds like over-complication to me. Is it solving a real problem that exists today with the `doall` approach?
Hide
Gary Fredericks added a comment -

I think the only reason to avoid that by default is the danger (is it really a problem?) of reading more records than the user needs. But it might be that the benefits of chunked seqs outweigh that risk, and just providing an option to unchunk would be sufficient.

Show
Gary Fredericks added a comment - I think the only reason to avoid that by default is the danger (is it really a problem?) of reading more records than the user needs. But it might be that the benefits of chunked seqs outweigh that risk, and just providing an option to unchunk would be sufficient.
Hide
Gary Fredericks added a comment -

It occurred to me that maybe a more efficient version of this would be the ability to (maybe not by default) construct a chunked-seq directly (in the lower-level manner) in the result-set-seq function. Would this address your objections?

Show
Gary Fredericks added a comment - It occurred to me that maybe a more efficient version of this would be the ability to (maybe not by default) construct a chunked-seq directly (in the lower-level manner) in the result-set-seq function. Would this address your objections?
Hide
Sean Corfield added a comment -

Most code out there is going to treat the result as a seq so I'd be concerned about potential overhead in changing from essentially `(seq (doall (map ...)))` to `(seq (vec (map ...)))`. Can you persuade me that's not a real issue?

Show
Sean Corfield added a comment - Most code out there is going to treat the result as a seq so I'd be concerned about potential overhead in changing from essentially `(seq (doall (map ...)))` to `(seq (vec (map ...)))`. Can you persuade me that's not a real issue?

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: