More small optimizations

[alexander-yakushev]

The first one saves a bit on that regexpy str/replace if the keyword already doesn't contain any hyphens. Alternatively, it could be rewritten into some explicit loop that looks for hyphens and then checks if they aren't on the edges of the string, but I don't think the complication is worth it. Checking for present hyphen is easy and quick, and given that most keywords don't have hyphens, this already saves most of what it is there to be saved.

The second one is some streamlining of code inside format-values. The refactored version tries to do most of work inside the reduce loop, without pre-processing stuff with map first.

[alexander-yakushev]

There is one testcase failing that checks that clause-reference.md example does not match the behavior of the PR; but unless I'm missing something obvious, the example appears not to be correct?

Where's 1 2 3 gone?

[seancorfield]

I'll take a look when I get back from vacation.

[seancorfield]

Took a quick look -- I think you're right: this is a bug in existing HoneySQL code, and it seems like your PR fixes it somehow?

Locally:

user=> (require '[honey.sql :as sql])
nil
user=> (System/getProperty "java.version")
"1.8.0_332"
user=> (sql/format {:insert-into :table
  #_=>                     :values [[1 2 3] :default [4 5 6]]})
["INSERT INTO table VALUES (?, ?, ?), DEFAULT, (?, ?, ?)" 1 2 3 4 5 6]
user=> (sql/format {:insert-into :table
  #_=>                     :values [{:a 1 :b 2 :c 3}
  #_=>                              :default
  #_=>                              {:a 4 :b 5 :c 6}]})
["INSERT INTO table (a, b, c) VALUES (?, ?, ?), DEFAULT, (?, ?, ?)" 6 5 4]
user=> 

I get that second, incorrect, result on every JDK locally.

[alexander-yakushev]

Here is the bug:

honeysql/src/honey/sql.cljc

Line 1206 in 8e0d698

(if params' (into params params') params')])

If params' is nil, it returns params' again (which is nil). It should've been params.

[seancorfield]

There's a reflection warning in util.cljc:

util.cljc:86 recur arg for primitive local: start is not matching primitive, had: Object, needed: long

I suspect you'll want to submit a PR to fix that?

[alexander-yakushev]

It's not a reflection warnings, but an autoboxing warning, and it doesn't matter that much here. But warnings are annoying, so I'm going to fix this, yeah.

[seancorfield]

Thanks. Shows up with (set! *warn-on-reflection* true) so I'm adding that to all source nses in HoneySQL, and I'll cut a new release shortly.