Avoid firing RemoveRedundantTableScanPredicate when there is no TableScan predicate #10549
Conversation
There was a problem hiding this comment.
Partially mitigates #10532
Which connectors it will be sufficient for?
I.e. will it help in real life, or only make testing harder?
Perhaps, instead of this, normalize expression before returning it from RemoveRedundantTableScanPredicate?
| @@ -57,7 +57,7 @@ | |||
|
|
|||
| private static final Pattern<FilterNode> PATTERN = | |||
| filter().with(source().matching( | |||
| tableScan().capturedAs(TABLE_SCAN))); | |||
| tableScan().capturedAs(TABLE_SCAN).matching(node -> !node.getEnforcedConstraint().isAll()))); | |||
There was a problem hiding this comment.
The rule code below doesn't make it obvious that this is a special case..
Maybe rename the rule to RemoveRedundantPredicateAboveTableScan
There was a problem hiding this comment.
Agreed. The name of the rule confused me when I started looking at it.
| @@ -57,7 +57,7 @@ | |||
|
|
|||
| private static final Pattern<FilterNode> PATTERN = | |||
| filter().with(source().matching( | |||
| tableScan().capturedAs(TABLE_SCAN))); | |||
| tableScan().capturedAs(TABLE_SCAN).matching(node -> !node.getEnforcedConstraint().isAll()))); | |||
There was a problem hiding this comment.
The rule code below doesn't make it obvious that this is a special case..
Maybe rename the rule to RemoveRedundantPredicateAboveTableScan
All connectors, especially when the filter contains expressions like the one above -- it's actually independent of connector implementation. It's a bug in how the Regardless, the rule shouldn't fire when there's no work to do, which is what I'm adding here. |
|
I've made PR that fixes this holistically: #10985 |
|
Folded into #10985 |
Partially mitigates #10532