liurenjie1024 merged PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309
--
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:
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2039348517
> Hi, @marvinlanhenke Thanks for pr, it looks great! I have some small
suggestion to restructure the code to make it easier for review. Really
greatful for these tests!
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1553275061
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,323 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1553272539
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,323 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1553271703
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,323 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1553262971
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,323 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
liurenjie1024 commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2039007641
cc @Fokko Do you have other comments?
--
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
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1548808318
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,323 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1547819858
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,300 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1547785953
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,300 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1547681855
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,300 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1547676828
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,300 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1547667177
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,300 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1547649993
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,300 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1547653811
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,300 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1547639860
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,300 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2031700958
> Hi, @marvinlanhenke Thanks for pr, it looks great! I have some small
suggestion to restructure the code to make it easier for review. Really
greatful for these tests!
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1547488931
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +269,300 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2029473672
> Hi, @marvinlanhenke I skimmed through the code and it seems that we only
need to split tests into separate modules to make it easier to maintain and
read. But I agree that we
liurenjie1024 commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2029468475
Hi, @marvinlanhenke I skimmed through the code and it seems that we only
need to split tests into separate modules to make it easier to maintain and
read. But I agree that we
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2029456431
> > @Fokko @liurenjie1024 PTAL
> > I ported all the tests - and fixed #311. Now, all tests are passing and
align with the Java implementation. I think we only can optimize
liurenjie1024 commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2029440339
> @Fokko @liurenjie1024 PTAL
>
> I ported all the tests - and fixed #311. Now, all tests are passing and
align with the Java implementation. I think we only can optimize
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2029426833
@Fokko @liurenjie1024
PTAL
I ported all the tests - and fixed #311. Now, all tests are passing and
align with the Java implementation.
I think we only can optimize
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2028045351
@Fokko @liurenjie1024
I'm having trouble understanding/ verifiying the test-case for
liurenjie1024 commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2026487818
Hi, @marvinlanhenke Thanks for your contribution, the overall design looks
good to me! Please take your time and enjoy time with family
--
This is an automated message from
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1543956147
##
crates/iceberg/src/transform/mod.rs:
##
@@ -37,6 +36,15 @@ pub trait TransformFunction: Send {
fn transform(, input: ArrayRef) -> Result;
///
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1543730206
##
crates/iceberg/src/spec/transform.rs:
##
@@ -398,6 +679,80 @@ mod tests {
}
}
+#[test]
+fn test_none_projection() -> Result<()> {
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2025357488
@liurenjie1024
I think I covered most of your suggestions.
PTAL if the overall design and implementation in general is fine?
### Unresolved Issues:
- [ ] How to
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542937628
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +270,174 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542923641
##
crates/iceberg/src/spec/values.rs:
##
@@ -683,6 +684,47 @@ impl Datum {
pub fn data_type() -> {
#type
}
+
+/// Create a new
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542923641
##
crates/iceberg/src/spec/values.rs:
##
@@ -683,6 +684,47 @@ impl Datum {
pub fn data_type() -> {
#type
}
+
+/// Create a new
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542916299
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +270,174 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542902827
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +270,174 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542870129
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +270,174 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542878163
##
crates/iceberg/src/transform/mod.rs:
##
@@ -16,6 +16,7 @@
// under the License.
//! Transform function used to compute partition values.
+
Review
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542877714
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +270,174 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542870129
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +270,174 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542837809
##
crates/iceberg/src/transform/mod.rs:
##
@@ -16,6 +16,7 @@
// under the License.
//! Transform function used to compute partition values.
+
Review
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542833995
##
crates/iceberg/src/spec/values.rs:
##
@@ -683,6 +684,47 @@ impl Datum {
pub fn data_type() -> {
#type
}
+
+/// Create a new `Datum`
liurenjie1024 commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2025035027
> Basically, I got rid of the trait and implemented boundary on Datum itself
- which makes more sense to me, now that I have a better overall picture with
the dates
liurenjie1024 commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542630137
##
crates/iceberg/src/spec/transform.rs:
##
@@ -261,6 +270,174 @@ impl Transform {
_ => self == other,
}
}
+
+/// Projects a
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2024950370
@Fokko
Is it correct to assume, as far as I understood the Java implementation,
that we support `dates` projection only for `year, month, and day`?
@liurenjie1024
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2024663637
> Thanks for picking this up @marvinlanhenke
>
> The most important part of adding projection is making that they are
absolutely correct. If rust would generate something
Fokko commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542486911
##
crates/iceberg/src/spec/transform.rs:
##
@@ -398,6 +679,80 @@ mod tests {
}
}
+#[test]
+fn test_none_projection() -> Result<()> {
+
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2024617450
> LGTM, with the caveat that I'm not an expert on this part of the spec, but
the PR seems well-structured.
>
> I wouldn't mind seeing more comments in the tests though to
marvinlanhenke commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542468150
##
crates/iceberg/src/spec/transform.rs:
##
@@ -398,6 +679,80 @@ mod tests {
}
}
+#[test]
+fn test_none_projection() -> Result<()> {
sdd commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542459274
##
crates/iceberg/src/spec/transform.rs:
##
@@ -398,6 +679,80 @@ mod tests {
}
}
+#[test]
+fn test_none_projection() -> Result<()> {
+
sdd commented on code in PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#discussion_r1542459274
##
crates/iceberg/src/spec/transform.rs:
##
@@ -398,6 +679,80 @@ mod tests {
}
}
+#[test]
+fn test_none_projection() -> Result<()> {
+
marvinlanhenke commented on PR #309:
URL: https://github.com/apache/iceberg-rust/pull/309#issuecomment-2023840212
@liurenjie1024 @ZENOTME @sdd PTAL
--
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
marvinlanhenke opened a new pull request, #309:
URL: https://github.com/apache/iceberg-rust/pull/309
### Which issue does this PR close?
Closes #264
### Rationale for this change
The ability to project row_filter to `Transform` partition.
This will unblock the Manifest &
50 matches
Mail list logo
