Propose a fragment concat operator that ensures a whitespace in between

[satorg]

This PR proposes a new operator +~+ for fragments concatenation that makes sure that there is at least 1 whitespace between the fragments being concatenated. I realize, this proposal is going to be a controversial one. However I decided to give it a shot.

Motivation

I've been working with Doobie for quite a while and use its fragment API to create pretty sophisticated queries. Some of those queries are created dynamically, i.e. various parts of the SQL are concatenated depending on some external conditions. With time I started getting a feeling that the whole fr/fr0/const/const0 paradigm might not be the great idea at all. When I create a fragment that may or may not be concatenated with something else later, I don't really want to worry about whether I have to end it with a whitespace or not. I do cary though about the whitespace at the moment when the concatenation happens, if it is the case. So this is where the idea of the proposed operator comes from: instead of keeping track of previous fragments endings, we can simply use fr0 and const0 everywhere along with either ++ or +~+ operators, depending on a particular case. For example:

// Consider it returns something like `SELECT ... FROM foo JOIN bar ON whatever`
def baseSelectSQL: Fragment = ???

def conditionalSelectSQL(one: Option[Int], two: Option[Int]) =
  baseSelectSQL +~+
    andOpt(
      one.map(a => fr0"one = $one"),
      two.map(b => fr0"two = $b)
    ).fold(Fragment.empty)(fr0"WHERE" +~+ _)

In other words, with +~+ operator we should never worry about adding explicit whitespaces anywhere anymore – the operator will take care of it automatically. Thereby the code becomes clearer and SQL logs (if employed) become less cluttered.

About the Naming

My initial intent was to give the operator this name: +_+. Unfortunately, Scala parser has its own business with underscores in operator-alike names and such a name cannot be used without backticks. So I ended up with something that is relatively simple and self-explanatory.

However, if there is a chance that the PR can be accepted, I'm generally open to any other (better) name, so feel free to suggest other options please.

[jatcwang]

Thanks for the PR! I agree that this is very useful.

[jatcwang]

I'm wondering whether we want to perform this check. Given the context it's used, I don't think extra spaces should matter?

(While working on fragments.scala, I did consider doing similar checks to generate nicer looking SQL but ultimately decided against it)

[satorg]

Sorry, I'm not sure I fully understand your question, do you mean to remove all the "if-s" and leave only the sql + " " + that.sql unconditionally?

My initial intent was that the checks are the main point of the new operator – it tries not to interfere with the spaces which a user might have already added and steps in only if they didn't, but at least one whitespace is still required. For example:

const0("SELECT a, b, c\nFROM the_table\n") +~+
  (if (we_need_cond) fr0("WHERE a = $a") else (Fragment.empty))

I.e. it should keep a new line before WHERE without more whitespaces added. So basically yes, it allows to format SQL nicely, when necessary. I personally do it for the sake of more readable logs, in particular.

But I realize it is not exactly a big deal, so let me know if it is actually a concern please.

[jatcwang]

Yeah that's what I meant.

I was trying to avoid the constant check on the last character of the string. Upon further digging I think it's negligible though. If you're often looking at logged DB queries (which I'm not) then the additional readability is beneficial, I agree 👍

[jatcwang]

Would you be able to take a look at src/main/mdoc/docs/08-Fragments.md and see where the docs can be updated to reflect this improvement? :)