a-agmon commented on PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#issuecomment-2348277213
Hi @liurenjie1024 ,
just fixed a few conflicts due to
https://github.com/apache/iceberg-rust/pull/594 (@FANNG1)
Should be good now
--
This is an automated message from the Ap
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1749162764
##
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##
@@ -0,0 +1,312 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or
a-agmon commented on PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#issuecomment-2336631735
Thank you very much for the review and comments, @liurenjie1024
I think I have addressed all your comments and questions and would be happy
if you can take a another look.
I
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1749163419
##
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##
@@ -0,0 +1,312 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1749163419
##
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##
@@ -0,0 +1,312 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1749163419
##
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##
@@ -0,0 +1,312 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1749163419
##
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##
@@ -0,0 +1,312 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1749162764
##
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##
@@ -0,0 +1,312 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1749162764
##
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##
@@ -0,0 +1,312 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or
liurenjie1024 commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1749056511
##
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##
@@ -0,0 +1,312 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+
a-agmon commented on PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#issuecomment-2325251287
Thanks much for the review and comments @liurenjie1024
I refactored the code to be based on `TreeNodeVisitor` now, and all unit
tests there.
I initially thought that `TreeNodeVi
liurenjie1024 commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1740552449
##
crates/integrations/datafusion/src/physical_plan/predicate_converter.rs:
##
@@ -0,0 +1,156 @@
+// Licensed to the Apache Software Foundation (ASF) under one
a-agmon commented on PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#issuecomment-2324171975
Hey @liurenjie1024 ,
I think this one is ready to go :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1740096736
##
crates/integrations/datafusion/src/physical_plan/scan.rs:
##
@@ -138,3 +147,223 @@ async fn get_batch_stream(
Ok(Box::pin(stream))
}
+
+/// Converts DataF
FANNG1 commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1740096447
##
crates/integrations/datafusion/src/physical_plan/scan.rs:
##
@@ -138,3 +147,223 @@ async fn get_batch_stream(
Ok(Box::pin(stream))
}
+
+/// Converts DataFu
a-agmon commented on PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#issuecomment-2323298188
> Aah I see! That makes complete sense. So, what you are saying is, we do
the best we can to filter via metadata within iceberg, but whatever operations
we can't handle in iceberg will
sdd commented on PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#issuecomment-2323295148
Aah I see! That makes complete sense. So, what you are saying is, we do the
best we can to filter via metadata within iceberg, but whatever operations we
can't handle in iceberg will get a
a-agmon commented on PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#issuecomment-2323283411
you can see what I mean in this test case:
```rust
#[test]
fn test_predicate_conversion_with_unsupported_condition_or() {
let sql = "(foo > 1 and bar in ('tes
a-agmon commented on PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#issuecomment-2323281873
> My main concern is that it seems like you have taken a "soft-fail"
approach, where, for unsupported operators or nodes, you return None and
proceed by effectively ignoring as-yet
a-agmon commented on PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#issuecomment-2323262045
Hey @sdd, @FANNG1 and @liurenjie1024
Please see updated PR. I have refactored `predicate_converter` into a
separate module that follows a more visitor-ish approach.
The `scan`
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1739863150
##
crates/integrations/datafusion/src/physical_plan/scan.rs:
##
@@ -138,3 +150,231 @@ async fn get_batch_stream(
Ok(Box::pin(stream))
}
+
+/// convert DataFu
sdd commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1739722426
##
crates/integrations/datafusion/src/physical_plan/scan.rs:
##
@@ -138,3 +150,231 @@ async fn get_batch_stream(
Ok(Box::pin(stream))
}
+
+/// convert DataFusion
a-agmon commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1738793217
##
crates/integrations/datafusion/src/table.rs:
##
@@ -82,6 +82,26 @@ impl TableProvider for IcebergTableProvider {
Ok(Arc::new(IcebergTableScan::new(
FANNG1 commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1738539115
##
crates/integrations/datafusion/src/physical_plan/scan.rs:
##
@@ -138,3 +150,231 @@ async fn get_batch_stream(
Ok(Box::pin(stream))
}
+
+/// convert DataFus
FANNG1 commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1738462529
##
crates/integrations/datafusion/src/table.rs:
##
@@ -82,6 +82,26 @@ impl TableProvider for IcebergTableProvider {
Ok(Arc::new(IcebergTableScan::new(
FANNG1 commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1738462529
##
crates/integrations/datafusion/src/table.rs:
##
@@ -82,6 +82,26 @@ impl TableProvider for IcebergTableProvider {
Ok(Arc::new(IcebergTableScan::new(
FANNG1 commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1738456745
##
crates/integrations/datafusion/src/table.rs:
##
@@ -82,6 +82,26 @@ impl TableProvider for IcebergTableProvider {
Ok(Arc::new(IcebergTableScan::new(
FANNG1 commented on code in PR #588:
URL: https://github.com/apache/iceberg-rust/pull/588#discussion_r1738455466
##
crates/integrations/datafusion/src/table.rs:
##
@@ -82,6 +82,26 @@ impl TableProvider for IcebergTableProvider {
Ok(Arc::new(IcebergTableScan::new(
a-agmon opened a new pull request, #588:
URL: https://github.com/apache/iceberg-rust/pull/588
This PR partially closes #585
- Adds datafusion filters to the `IcebergTableScan` struct
- Converts datafusion filters (or `Exp`) to Iceberg `Predicate` and apply
them to the TableScan On dat
29 matches
Mail list logo
