Skip to content

Conversation

@geooo109
Copy link
Collaborator

@geooo109 geooo109 commented Oct 20, 2025

Fixes #6137

We should avoid simplifying cases like SELECT 1 AND x or SELECT 0 or x.
So, this PR makes this check tighter by checking if the LHS or RHS is Boolean type.

@georgesittas
Copy link
Collaborator

Discussed offline– we should check whether all engines evaluate connectors into boolean values, instead of treating them as control flow. If SELECT <always true value> AND other is always boolean, we can't just simplify it to SELECT other, because the semantics change. Unfortunately, this is what we do today, which is incorrect.

@geooo109 geooo109 changed the title fix(optimizer)!: simplify AND with static BOOLEAN type fix(optimizer)!: simplify AND/OR with static BOOLEAN type Oct 21, 2025
@geooo109 geooo109 force-pushed the geooo109/simplify_and branch from 25beb8c to 306b884 Compare October 21, 2025 14:50
@geooo109 geooo109 force-pushed the geooo109/simplify_and branch from 306b884 to 66ffdd7 Compare October 22, 2025 12:55
@georgesittas georgesittas marked this pull request as draft October 23, 2025 09:49
@georgesittas
Copy link
Collaborator

We discovered that post-simplifying we lose some types. That is a bug and we decided to address it in this PR instead of postponing, because that will reduce the PR's complexity as a side-effect.

@georgesittas
Copy link
Collaborator

@barakalon @tobymao curious what your thoughts are on this.

The problem: today we infer types & then run other rules that transform the AST, resulting in building new, untyped, expressions. This is a bug on its own, and it also affects type-sensitive rules. E.g., simplify now checks the boolean type to apply certain transformations but inbetween types are lost and so it cannot always work.

My worry is that fixing each call site introduces a new pattern where we have to be alert for rules after annotate_types and ensure new AST nodes are always typed. OTOH, not sure how else we can unblock these rules in simplify that run up to a fix point...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The optimizer changes the query's result from a boolean value to the column's raw value

3 participants