liurenjie1024 commented on issue #159: URL: https://github.com/apache/iceberg-rust/issues/159#issuecomment-1888538588
I think we have reached consensus that storing a `BoundLiteral` in `UnboundExpression` is acceptable. About the visibility of this struct, I would argue that we still need to expose it user, for following reason: 1. When user construction `UnboundExpression`, it's not always from sql. For example when we integrate it with other sql engines, it may push filter to iceberg api to construct `UnboundExpression`. 2. The parse string method implemented in `pyiceberg` is not a typical approach in rust. Rust has elegant support for macros, which is efficient and type safe. So my proposal is that we still make `BoundLiteral` public, but we should be careful not exposing its internals in public api. For the first version, we'll only expose builder methods so that users can create it easily, and no internals will be exposed. WDYT? cc @Fokko @ZENOTME @Xuanwo -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
