fix: not do boolean folding on NULL and/or expr

[NGA-TRAN]

Which issue does this PR close?

Closes #1244

Rationale for this change

Do not fold Boolean(NULL) and/or expr because it depends on the value of the expr

What changes are included in this PR?

Boolean(NULL) and/or expr is key as-is (aka not rewrite)

Are there any user-facing changes?

This bug was hit by IOx's storage_api::test

[NGA-TRAN]

@alamb and @matthewmturner Can you help review this?

[xudong963]

Thanks for your nice work 🎉@NGA-TRAN

FYI

case1
postgres=# select 11 BETWEEN 0 AND 10 and NULL as c;
 c
---
 f
(1 row)

case2
postgres=# select 1 BETWEEN 0 AND 10 and NULL as c;
 c
---

(1 row)

case3
postgres=# select 1 BETWEEN 0 AND 10 or NULL as c;
 c
---
 t
(1 row)

case4
postgres=# select 11 BETWEEN 0 AND 10 or NULL as c;
 c
---

(1 row)

So I think we can add two tests about case1 and case4. @NGA-TRAN

[alamb]

Thank you @NGA-TRAN -- I find the new structure and comments much easier to follow.

[alamb]

FWIW I am working on getting these constant cases to work with the ConstEvaluator (so they wouldn't be strictly necessary) but it turns out these types of expressions don't actually work in the general evaluator

I am working on it in #1163

But in any event I think this code is good 👍

[NGA-TRAN]

@xudong963 Those are great queries. I have tried to add them in the test but I think we have not done constant folding for Between yet (or at least not for the queries in your examples) so they are not folded at this time. Hence, I do not commit them in because they are the same as the current between query.

I can open a new ticket for improving these cases though. What do you think @alamb ?

[NGA-TRAN]

@alamb I have addressed the comments. It should be ready for merge when all checks pass

[alamb]

@xudong963 Those are great queries. I have tried to add them in the test but I think we have not done constant folding for Between yet (or at least not for the queries in your examples) so they are not folded at this time. Hence, I do not commit them in because they are the same as the current between query.

I can open a new ticket for improving these cases though. What do you think @alamb ?

I think the constant folding is handled by ConstEvaluator (in a different module). The Simplifier is supposed to be focused on items that can't be folded (like TRUE OR <expr> --> <expr>).

The fact we have some boolean constant evaluation in here is a historical artifact and due to the fact we don't have full expression evaluation support for all boolean operations (yet).

Thus I don't think a separate ticket is needed

[alamb]

Thanks @NGA-TRAN