Replace String.format usage with simple string concatenation

[wendigo]

No description provided.

[hashhar]

this seems more like convenience rather than required? You can surely append a trailing space to each line.

I'll look into how Joiner is implemented - maybe I'm overthinking.

[ksobolew]

Is this for performance, perhaps? I don't find it more readable or more convenient...

[wendigo]

Actually it is more readable as with the format you need to understand which argument goes where

[hashhar]

Joiner is actually more performant and less garbage generating compared to even concat since it uses good old StringBuilder underneath and some intelligent logic.

Yes, this is for perf.

[ksobolew]

Don't underestimate plain concatenation, in Java 9+ it no longer desugars to StringBuilder and uses some INVOKEDYNAMIC magic instead :)

[ksobolew]

(I'm not blocking this PR, BTW, do what you have to do)

[hashhar]

Yes, I tested with a JMH harness, Joiner is still faster.

[hashhar]

wait a sec, my measurements were wrong.

Plain concat is the fastest indeed.
JOINER is MUCH faster than format.
format must be avoided.

So if plain concat preserves readability (it seems to do) then let's use it.

For me joiner won because my JMH was generating random inputs for each method being benchmarked instead of each run having same input.

[hashhar]

Benchmark                     Mode  Cnt    Score    Error  Units
DifferentConcats.concat       avgt   15  143.218 ±  4.177  ns/op
DifferentConcats.guavaJoiner  avgt   15  398.257 ± 12.523  ns/op
DifferentConcats.formatStr    avgt   15  750.233 ± 16.783  ns/op

There's a steady 2x improvement between each approach.

[martint]

For this case, readability matters more than performance. How many of these operations are being performed per Trino query?

[hashhar]

Soft LGTM.

I'll look at Joiner impl and give a green light/request changes.

[hashhar]

LGTM