You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "liurenjie1024 (via GitHub)" <gi...@apache.org> on 2024/07/29 14:06:27 UTC
[I] refactor: Simplify `PrimitiveLiteral` type. [iceberg-rust]
liurenjie1024 opened a new issue, #501:
URL: https://github.com/apache/iceberg-rust/issues/501
We have introduce `Datum` in recent prs, which associate data type with a primitive literal. This means that `PrimitiveLiteral` is supposed to be a physical value, and we no longer need so many variants (such as date, time). More discussion could be found in https://github.com/apache/iceberg-rust/pull/492.
I'm here to propose to simplify it as following:
```rust
pub enum PrimitiveLiteral {
/// 0x00 for false, non-zero byte for true
Boolean(bool),
/// Stored as 4-byte little-endian
Int(i32),
/// Stored as 8-byte little-endian
Long(i64),
/// Stored as 4-byte little-endian
Float(OrderedFloat<f32>),
/// Stored as 8-byte little-endian
Double(OrderedFloat<f64>),
/// UTF-8 bytes (without length)
String(String),
/// Binary value (without length)
Binary(Vec<u8>),
/// Stores unscaled value as big int. According to iceberg spec, the precision must less than 38(`MAX_DECIMAL_PRECISION`) , so i128 is suit here.
Decimal(i128),
}
```
--
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-unsubscribe@iceberg.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-2256129508
Thanks for this. I support this change,
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-2256323789
> This would make a lot of sense. I was also never keen on the names. Maybe we could rename Datum to `LogicalValue` and have `PhysicalValue` for primitive, similar to Parquet?
I prefer to reserve `Primitive` in name to indicate the primitive value, e.g `LogicalPrimitive` `PhysicalPrimitive`
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed issue #501: discussion: Simplify `PrimitiveLiteral` type.
URL: https://github.com/apache/iceberg-rust/issues/501
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] refactor: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-2256043309
cc @Xuanwo @ZENOTME @Fokko @sdd
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-2256319932
I create an init PR #502 for this. I hope it can be a starting point to discuss what's going on if we simplify PrimitiveLiteral. So feel free for any suggestions.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-3358582898
This issue has been automatically marked as stale because it has been open for 180 days with no activity. It will be closed in next 14 days if no further activity occurs. To permanently prevent this issue from being considered stale, add the label 'not-stale', but commenting on the issue is preferred when possible.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-3408703303
This issue has been closed because it has not received any activity in the last 14 days since being marked as 'stale'
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "sdd (via GitHub)" <gi...@apache.org>.
sdd commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-2256250188
This would make a lot of sense. I was also never keen on the names. Maybe we could rename Datum to `LogicalValue` and have `PhysicalValue` for primitive, similar to Parquet?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "sdd (via GitHub)" <gi...@apache.org>.
sdd commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-2256252671
Actually if we did that it would diverge from the Iceberg spec
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-2256141727
Also +1 for this and I'm glad to send the PR to do this later if we decide.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] discussion: Simplify `PrimitiveLiteral` type. [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #501:
URL: https://github.com/apache/iceberg-rust/issues/501#issuecomment-2258524166
> > This would make a lot of sense. I was also never keen on the names. Maybe we could rename Datum to `LogicalValue` and have `PhysicalValue` for primitive, similar to Parquet?
>
> I prefer to reserve `Primitive` in name to indicate the primitive value, e.g `LogicalPrimitive` `PhysicalPrimitive`
+1 for this. Also we may don't need to actually rename it, just adding an alias would be enough.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org