search

cmu-db / optd

CMU-DB's Cascades optimizer framework
https://cmu-db.github.io/optd/
MIT License
373 stars 22 forks source link

Fix: fix for heuristic rule wrapper #147

Closed AveryQi115 closed 7 months ago

AveryQi115 commented 7 months ago

The new expr returned by heuristic rules are not in the original group, which means it never got an OptimizeExpressionTask with exploring as false (OptimizeExpressionTask with exploring=False only be called in OptimizeGroup), it should evoke an OptimizeExpressionTask with exploring=false for itself to apply all the transform rules and implementation rules for itself.

Besides, the pr adds checks for whether the original expr equals to the output expr for heuristic rule. If that's the case, it should prompt an error as this breaks the heuristic rule's definition. (Silently accepting it will mark the input expr being as a dead end and there's no more new exprs to replace it)

skyzh commented 7 months ago

did we consider merge two groups of the same ID in cascades? maybe we also need to fix that logic.

AveryQi115 commented 7 months ago

did we consider merge two groups of the same ID in cascades? maybe we also need to fix that logic.

I've thought about this case. But I don't think we need for this one. As the input two groups for merge group are not newly generated by apply_rule task. They exist before the apply_rule task. So there must be optimizeGroup task for them already in the task list. And the exprs in the two groups then have been evoked OptimizeExpression task, even they are moved from one group to another group?

AveryQi115 commented 7 months ago

did we consider merge two groups of the same ID in cascades? maybe we also need to fix that logic.

I've thought about this case. But I don't think we need for this one. As the input two groups for merge group are not newly generated by apply_rule task. They exist before the apply_rule task. So there must be optimizeGroup task for them already in the task list. And the exprs in the two groups then have been evoked OptimizeExpression task, even they are moved from one group to another group?

But that is based on the assumption that the rules returning a group cannot create new groups in rules logic, they can only return groups that already existed(for example, a rule is extracting filter node and its child and only returns its child node and eliminate the filter)

AveryQi115 commented 7 months ago

Okay, we probably need that fix for merge group too then. The optimizeGroup task is only evoked for the root relnode, and other optimzieGroup task are called from optimizeInput(when it wants to optimize its children group). If a child group after some rule application is merged to a top level group which is already called OptimizeGroup task, the optimize group task needs to be optimized again for its newly added expressions.