datafusion icon indicating copy to clipboard operation
datafusion copied to clipboard

Check Cast Expr Before Adding Equivalences

Open berkaysynnada opened this issue 10 months ago • 2 comments

Which issue does this PR close?

Closes #9832.

Rationale for this change

Casted columns may lost their ordering. We need check the source and target datatypes to ensure ordering still exists.

What changes are included in this PR?

get_ordering() API of PhysicalExpr's get an optional schema. It only looks up it for cast expressions but I cannot figure out simpler way to get the source datatype of the casted expr. update_ordering() and get_expr_ordering() have been changed for this end.

Are these changes tested?

Yes, added an .slt

Are there any user-facing changes?

berkaysynnada avatar Mar 29 '24 08:03 berkaysynnada

Is this PR ready to merge?

alamb avatar Apr 03 '24 21:04 alamb

Is this PR ready to merge?

We are thinking on more extendable and user friendly API. Giving schemas as an option can be a misuse, and in the future, we may need to know more information about the children, such as range. When we agree on this, I will re-open.

berkaysynnada avatar Apr 04 '24 07:04 berkaysynnada