refactor: iceberg::spec::values::Struct to remove bitvec #1203
Conversation
Motivation: apache#1000 (comment) Signed-off-by: xxchan <xxchan22f@gmail.com>
| aws-config = "1" | ||
| aws-sdk-glue = "1.39" | ||
| bimap = "0.6" | ||
| bitvec = "1.0.1" |
There was a problem hiding this comment.
Thank for this change, but funty is maintained by the same team of bitvec and is not updated for 2 years.
Signed-off-by: xxchan <xxchan22f@gmail.com>
liurenjie1024
left a comment
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @xxchan for this pr, LGTM!
| # Some dependencies don't correctly specify a minimal version for their dependencies and will fail to build. | ||
| # So we update these transitive dependencies here. | ||
| cargo update tap faststr metainfo linkedbytes | ||
| cargo update faststr metainfo linkedbytes |
There was a problem hiding this comment.
Not related to this pr, but I don't think updating this in ci is a good practice. If we require minimum version of some package, why not just maintaining them specifically in our Cargo.toml?
| fields: Vec<Literal>, | ||
| /// Null bitmap | ||
| null_bitmap: BitVec, | ||
| fields: Vec<Option<Literal>>, |
There was a problem hiding this comment.
In theory, the memory used in this approach is a little larger, but I think it's fine to do it for now.
There was a problem hiding this comment.
Option enum usually have the same memory size as the enum
|
Wait for @Xuanwo to take another look. |
Xuanwo
left a comment
Xuanwo
left a comment
There was a problem hiding this comment.
Thank you for working on this!
## Which issue does this PR close? - remove tap, it's not needed any more after #1203. Added by mistake in #1194 - remove cargo update in minimal versions gen, since we put them in Cargo.toml now. (#1203 (comment)) - bump reqwest, bytes to newer version to resolve some minimal version build issue (not latest) - Closes #. --------- Signed-off-by: xxchan <xxchan22f@gmail.com>
3 participants
Motivation: #1000 (comment)
Signed-off-by: xxchan xxchan22f@gmail.com
Which issue does this PR close?
What changes are included in this PR?
Are these changes tested?