Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

marvinlanhenke commented on PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#issuecomment-2066180837 > This looks great! > > I think @marvinlanhenke has a valid concern on when to apply the rewrite-not. Let's defer that discussion when we start wiring everything

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

Fokko merged PR #335: URL: https://github.com/apache/iceberg-rust/pull/335 -- 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:

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd commented on code in PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#discussion_r1571889478 ## crates/iceberg/src/expr/visitors/inclusive_projection.rs: ## @@ -0,0 +1,371 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd commented on PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#issuecomment-2065786584 I agree that rewrite-not should get applied. But it is not the responsibility of the `InclusiveProjection` itself. In this design, it happens already earlier on on the process, at the

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

marvinlanhenke commented on code in PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#discussion_r1570491035 ## crates/iceberg/src/expr/visitors/inclusive_projection.rs: ## @@ -0,0 +1,371 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd commented on PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#issuecomment-2063172829 FAO @Fokko @marvinlanhenke @liurenjie1024: Comments addressed, ready for re-review -- This is an automated message from the Apache Git Service. To respond to the message, please log on

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd commented on code in PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#discussion_r1570118932 ## crates/iceberg/src/expr/visitors/inclusive_projection.rs: ## @@ -0,0 +1,371 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd commented on code in PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#discussion_r1570116819 ## crates/iceberg/src/expr/visitors/inclusive_projection.rs: ## @@ -0,0 +1,371 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd commented on code in PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#discussion_r1570115947 ## crates/iceberg/src/expr/visitors/inclusive_projection.rs: ## @@ -0,0 +1,371 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

Fokko commented on code in PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#discussion_r1568751884 ## crates/iceberg/src/expr/visitors/inclusive_projection.rs: ## @@ -0,0 +1,371 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

Fokko commented on code in PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#discussion_r1568763855 ## crates/iceberg/src/expr/visitors/inclusive_projection.rs: ## @@ -0,0 +1,371 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

marvinlanhenke commented on code in PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#discussion_r1568705676 ## crates/iceberg/src/expr/visitors/inclusive_projection.rs: ## @@ -0,0 +1,371 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd commented on PR #335: URL: https://github.com/apache/iceberg-rust/pull/335#issuecomment-2059682467 @liurenjie1024 @Fokko @marvinlanhenke @ZENOTME PTAL, I've refactored this on top of the `BoundPredicateVisitor` that was merged a few hours ago and it is ready for review -- This is an

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd commented on PR #321: URL: https://github.com/apache/iceberg-rust/pull/321#issuecomment-2059636629 Closing this one in favour of the alternate PR based on the BoundPredicateVisitor design that got merged (https://github.com/apache/iceberg-rust/pull/335) -- This is an automated

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd closed pull request #321: add `InclusiveProjection` Visitor URL: https://github.com/apache/iceberg-rust/pull/321 -- 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

Re: [PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd commented on PR #321: URL: https://github.com/apache/iceberg-rust/pull/321#issuecomment-2041438110 Updated to align with latest `BoundPredicateVisitor` iteration in https://github.com/apache/iceberg-rust/pull/320 -- This is an automated message from the Apache Git Service. To

[PR] add `InclusiveProjection` Visitor [iceberg-rust]

sdd opened a new pull request, #321: URL: https://github.com/apache/iceberg-rust/pull/321 This PR has been broken out of https://github.com/apache/iceberg-rust/pull/241 as it was getting too large. The `InclusiveProjection` is used in the process of filtering manifest files in table