SQL: Improve the optimization of null conditionals #71192
Conversation
Enhance the existing rules so that Coalesce(ex) -> ex NullIf(a, a) -> null NullIf(null, a) -> null NullIf(a, null) -> a
|
Pinging @elastic/es-ql (Team:QL) |
|
|
||
| @Override | ||
| public boolean foldable() { | ||
| return left.semanticEquals(right) || super.foldable(); |
There was a problem hiding this comment.
Shouldn't also be foldable if left.foldable() and left.fold() == null, but then we need to call left.fold() so I'm not sure we should do that.
There was a problem hiding this comment.
That's covered by the optimizer rule below: https://github.com/elastic/elasticsearch/pull/71192/files#diff-f89f100e88759827c9f9cbceed83a4efb475521a3e42d489584917807df11190R702
There was a problem hiding this comment.
but then we need to call left.fold() so I'm not sure we should do that.
@matriv fmi, why would that be problematic?, if .foldable()?
There was a problem hiding this comment.
Conceptually you call foldable() beforehand, to check if it's safe to fold() something, so it's weird to call fold() on a child inside the check of foldable().
| Expression firstChild = children.get(0); | ||
| boolean sameChild = true; | ||
| for (int i = 1; i < children.size(); i++) { | ||
| Expression child = children.get(i); |
There was a problem hiding this comment.
Nit: there's an extra space after equals sign, but the assignment could also be done away with, child's only used once in following expression.
|
|
||
| @Override | ||
| public boolean foldable() { | ||
| return left.semanticEquals(right) || super.foldable(); |
There was a problem hiding this comment.
but then we need to call left.fold() so I'm not sure we should do that.
@matriv fmi, why would that be problematic?, if .foldable()?
Enhance the existing rules so that Coalesce(ex) -> ex NullIf(a, a) -> null NullIf(null, a) -> null NullIf(a, null) -> a
Enhance the existing rules so that Coalesce(ex) -> ex NullIf(a, a) -> null NullIf(null, a) -> null NullIf(a, null) -> a
Enhance the existing rules so that
Coalesce(ex) -> ex
NullIf(a, a) -> null
NullIf(null, a) -> null
NullIf(a, null) -> a