You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/05/25 17:58:15 UTC

[GitHub] [arrow-datafusion] alamb commented on issue #6437: Inconsistencies in `Expr` operator code.

alamb commented on issue #6437:
URL: https://github.com/apache/arrow-datafusion/issues/6437#issuecomment-1563296056

   > In my opinion, the Expr impl should not duplicate built-in operator trait methods. The duplicated methods should be removed from Expr, and only the trait impls be kept. This is more in line with idiomatic Rust code, removes duplication, and reduces potential user confusion.
   
   I agree a single Rust idomatic pattern sounds good to me. 
   
   I wonder if it might cause issues for python bindings, or other non rust users -- @andygrove  or @jdye64  do you have any insight?
   
   Thank you for working on cleaning this up 🙏 
   
   > Likewise, the Not trait impl should be moved to the operator module and the corresponding type method be removed.
   
   I agree


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org