Fokko commented on code in PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#discussion_r1836605349
##
crates/iceberg/src/spec/table_metadata.rs:
##
@@ -222,21 +222,26 @@ impl TableMetadata {
/// Returns all partition specs.
#[inline]
-pub fn partitio
liurenjie1024 merged PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645
--
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...@ic
liurenjie1024 commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2453287370
Thanks @c-thiel for this great pr!
--
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
liurenjie1024 commented on code in PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#discussion_r1826899584
##
crates/iceberg/src/spec/partition.rs:
##
@@ -131,50 +205,22 @@ impl PartitionSpec {
/// * Field names
/// * Source column ids
/// * Transform
liurenjie1024 commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2453287068
> @Xuanwo, @liurenjie1024 ready for another round. Ditched both the trait
and the enum. Bare with me:
>
> As already mentioned earlier, the enum or the trait were just a ve
c-thiel commented on code in PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#discussion_r1825848112
##
crates/iceberg/src/spec/partition.rs:
##
@@ -131,50 +205,22 @@ impl PartitionSpec {
/// * Field names
/// * Source column ids
/// * Transforms
-
liurenjie1024 commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2451776760
Also please merge with main branch to keep it updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use th
liurenjie1024 commented on code in PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#discussion_r1825726996
##
crates/iceberg/src/spec/partition.rs:
##
@@ -235,6 +281,21 @@ impl UnboundPartitionSpec {
}
}
+fn has_sequential_ids(field_ids: &[i32]) -> bool {
R
c-thiel commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2442922625
@Xuanwo, @liurenjie1024 ready for another round.
Ditched both the trait and the enum. Bare with me:
As already mentioned earlier, the enum or the trait were just a vehicle to
c-thiel commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2435124830
Enum it is then. I'll refractor it by the end of this week.
Thanks @Xuanwo @liurenjie1024!
--
This is an automated message from the Apache Git Service.
To respond to the message, p
liurenjie1024 commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2434867189
> I believe using `enum` here is sufficient. It's acceptable for us to have
some duplicated code since we know the APIs we need are limited and unlikely to
expand significantly i
Xuanwo commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2434748618
I believe using `enum` here is sufficient. It's acceptable for us to have
some duplicated code since we know the APIs we need are limited and unlikely to
expand significantly in the fut
liurenjie1024 commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2434716448
Hi, @c-thiel
> Regarding 1) they are not identical (see also my argument from the start.
FieldId is optional for unbound spec, but it is not for "previously bound"
partit
c-thiel commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2431923524
Hey @liurenjie1024 , thanks for giving it a shot.
Regarding 1) they are not identical (see also my argument from the start.
FieldId is optional for unbound spec, but it is not for "p
liurenjie1024 commented on code in PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#discussion_r1812191734
##
crates/iceberg/src/spec/partition.rs:
##
@@ -54,22 +54,51 @@ impl PartitionField {
}
}
-/// Partition spec that defines how to produce a tuple of p
c-thiel commented on code in PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#discussion_r1799542924
##
crates/iceberg/src/spec/partition.rs:
##
@@ -235,6 +261,179 @@ impl UnboundPartitionSpec {
}
}
+/// Trait for common functions between [`PartitionSpec`],
Xuanwo commented on code in PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#discussion_r1799004933
##
crates/iceberg/src/spec/partition.rs:
##
@@ -235,6 +261,179 @@ impl UnboundPartitionSpec {
}
}
+/// Trait for common functions between [`PartitionSpec`],
Xuanwo commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2402617523
Hi, @c-thiel, thanks a lot for working on this. This issue does seem
complex. I didn't join the discussion before, so I will take some time to go
through all the references and then rev
c-thiel commented on PR #645:
URL: https://github.com/apache/iceberg-rust/pull/645#issuecomment-2376026264
Introducing `SchemalessPartitionSpec` might be our way to avoid
https://github.com/apache/iceberg/issues/4563.
--
This is an automated message from the Apache Git Service.
To respond
c-thiel opened a new pull request, #645:
URL: https://github.com/apache/iceberg-rust/pull/645
Fixes https://github.com/apache/iceberg-rust/issues/550
This PR is a result of the issue mentioned above, a [Slack Discussion](
https://apache-iceberg.slack.com/archives/C03LG1D563F/p17258788
20 matches
Mail list logo
