Folks, I'm looking at https://github.com/seancorfield/honeysql/issues/374 and trying to figure out the best path forward here. The TL;DR is that HoneySQL has always treated :is as a synonym for := and :is-not as a synonym for :<> which is not quite correct. In addition, equality and inequality when one operand or the other is nil is special-cased and always produces IS NULL or IS NOT NULL as appropriate -- which means that [:= a :col] when a evaluates to nil produces col IS NULL but when a evaluates to, say, 42 produces 42 = col (well, ? = col and a parameter of 42).

At issue here is that IS/`IS NOT` are not synonyms for =/`<>` for Boolean operands or situations where the second operand is nil (`NULL`). I verified this on MySQL (in addition to what's in that issue).

So this is definitely a bug but I'm concerned that fixing it is going to silently change the behavior of existing code.

The existing behavior with =/`<>` and one operand being nil is intended to be a convenience, so you can just write [:= :col foo] and get either col = ? (and the parameter value of foo) or col IS NULL depending on the value of foo. Checking col = NULL here would always be an empty result.

I think that behavior should be retained because, well, it's convenient and I suspect quite a bit of code out there relies on that happening.

But that aliasing behavior is not correct for Boolean columns (`BIT(1)` in MySQL) when testing for TRUE or FALSE (or UNKNOWN but that is not yet supported by HoneySQL) -- because col IS NOT TRUE and col <> TRUE behave differently if col can be NULL.

(and right now there's no way to get the correct IS / IS NOT behavior for Boolean columns because HoneySQL turns those into = and <> respectively)

If I just remove the aliasing (which would be "correct"), any existing code using :is or :is-not is potentially going to change behavior in a subtle way (which is worse than just "breaking").

(note that [:= a :col] or [:is a :col] transforms to col IS NULL if a happens to be nil but NULL = col produces no matches and NULL IS col is a syntax error so that is a special case that is also problematic)

I think, clearly, whatever I do has to have at the very least a dynamic var that folks can set to select the behavior. We certainly have code at work that explicitly relies on being able to do [:= :col nil] / [:<> :col nil] and get col IS NULL / col IS NOT NULL so I'm loathed to change that behavior.

The first time I used honeysql I had to triple check about this. In the end, I came to like the := behavior because I thought it was just a nice way when dealing with explicit nils. Especially when passing in a literal nil.

I think that is/is-not should be left as is though. Clearly the caller knows what they're doing.

[:is a :col] should probably produce correct sql, ordering wise and/or throw if a is not nil.

Thanks @orestis That's useful feedback. I think I am beginning to lean toward just removing the aliasing for :is/`:is-not` and warning folks in the change log about this (and updating the docs in several places to show examples of this behavior and the subtle differences).

Check your code for :is-not uses!

We'll see whether it actually breaks any real code...