liurenjie1024 commented on issue #159: URL: https://github.com/apache/iceberg-rust/issues/159#issuecomment-1884133592
> What do you mean by self-contained? For example, why is a date self-contained, and a decimal not? Sorry for the unclear description. Currently the decimal value is stored in an `Decimal(i128)`, that means we lost the scale in type. (Think about the case we want to print it for error reporting, we can't correctly print it, but it's not the case the `Date(i32)`. In rust api, the `Literal` doesn't contains complete type info. Compared with python/java api, `Literal` is used in more places, for example, in the upper/lower bounds of partition field summary, [rust uses `Literal`](https://github.com/apache/iceberg-rust/blob/7d06a850bef5fcc1752977f109d343ddab26c5c7/crates/iceberg/src/spec/manifest_list.rs#L635) rather than bytes. This way we can do some check and avoid unsafe codes. > I think this goes back to an earlier discussion: > > pub enum Datum { > bool(bool), > int(i32), > ... > datetime(DateTime<Utc>), > decimal(Decimal), > } Yes, but this `Datum` serves for different purpose, it's mainly user facing api to construct expression. > The Datum seems like Literal with type info, so I think a new representationš¤: > > struct Datum { > ty: DataType, > value: Literal > } This sounds a good idea to me, which doesn't introduce extra maintain effort. The only concern is that it maybe not quite user friendly, but it seems that we can add some api to make it easier to use. -- 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
