Fix potential infinite loop at IterativeOptimizer with Iceberg#20007
Fix potential infinite loop at IterativeOptimizer with Iceberg#20007raunaqmorarka merged 1 commit intotrinodb:masterfrom
Conversation
assaf2
requested review from
Praveen2112,
alexjo2144,
findepi,
marton-bod,
raunaqmorarka and
sopel39
and removed request for
findepi
sopel39
left a comment
sopel39
left a comment
There was a problem hiding this comment.
lgtm % but I would like someone more familiar with code to take a look. What about test?
marton-bod
left a comment
marton-bod
left a comment
There was a problem hiding this comment.
LGTM - @zhangminglei can you also take a look? This concerns the change to the applyFilter method
| if (newEnforcedConstraint.equals(table.getEnforcedPredicate()) | ||
| && newUnenforcedConstraint.equals(table.getUnenforcedPredicate()) | ||
| && newConstraintColumns.equals(table.getConstraintColumns()) | ||
| && constraint.getPredicateColumns().isEmpty()) { |
There was a problem hiding this comment.
@marton-bod / @zhengxingmao - What was the initial reason for adding a check on constraint.getPredicateColumns().isEmpty() - Ideally the applyFilter should depends on the IcebergTableHandle state after applying the filter and not based on Constraint's state
I don't know why this condition was added in the first place, so since I don't see its necessity, I'm not sure how to test its removal |
5 participants
Description
Remove a condition that can potentially lead to an infinite loop at
IterativeOptimizer.When this condition was added,
constraint.getPredicateColumns()was always empty when the rule was running within iterative optimizer (see discussion at https://github.com/trinodb/trino/pull/17263/files#r1189575899) but this is not the case since https://github.com/trinodb/trino/pull/16019/files#diff-624ea155d75175ba8802d5f93af759bdb88c6f63f1ca9660d6704892e1033f1dR814-R825. According to my check, we don't see an infinite loop only becauseio.trino.sql.planner.iterative.rule.PushPredicateIntoTableScan#arePlansSamereturnstrueand stops the iterative process. If in the futureio.trino.plugin.iceberg.IcebergTableHandle#equalswill returnfalsewhen comparing 2 tables of 2 different iterations, then we'll see an infinite loop. That's whileio.trino.spi.connector.ConnectorTableHandleis an empty interface that doesn't declareequals().Additional context and related issues
19876
Release notes
( X ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: