ZENOTME commented on PR #287: URL: https://github.com/apache/iceberg-rust/pull/287#issuecomment-2011068196
> @ZENOTME Thank you for this PR. I think I can work with that. > > One question though, regarding the input: `&Literal` - I guess we do this, due to the greater flexibility? > > However, for #264 I'm working with `BoundPredicates` as input. > > This means I'd still have to expose the underlying `Datum` and its `PrimitiveLiteral` in order to use `fn transform_literal`. Also, when creating a new `UnboundPredicate` I'd have to construct a new `Datum` from the transformed literal. > > If we don't need the extra flexibility, changing `fn transform_literal` to > > ```rust > fn transform_literal(&self, input: &Datum) -> Result<Option<Datum>> > ``` > > would make the implementation of #264 a lot easier. Perhaps it should be named transform_datum then. > > Anyway, thanks again - I'd appreciate your thoughts on this. For now, `transform` does not rely on the type info, so it can do transform without type info. And `transform_literal` will also be used in manifest metadata, and [manifest store the literal directly](https://github.com/liurenjie1024/iceberg-rust/blob/470e6f08e1291df2bc2ecd22ae9415851beea6ba/crates/iceberg/src/spec/manifest_list.rs#L674). So I think in here what we need is a function extract the reference of literalš¤ -- 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
