liurenjie1024 commented on PR #241:
URL: https://github.com/apache/iceberg-rust/pull/241#issuecomment-2031946776
> @liurenjie1024 @ZENOTME @sdd @Xuanwo I'd really appreciate your thoughts
on this:
>
> I took a closer look at the work @sdd has already done - and I think in
order to proceed it would make sense to split the `ManifestEvaluator`,
`InclusiveProjection` and `PartitionEvaluator` into separate modules.
>
> I am thinking about putting each visitor into its own file within
`/iceberg/src/expr` (perhaps even another subfolder /visitors) - for example:
`/iceberg/src/expr/manifest_evaluator.rs.`
>
> This way, we could create individual issues on each implementation - and
work better in parallel.
>
> Perhaps, it also makes sense to provide a trait to make sure each visitor
adheres to the same interface (although I'm not sure this is neccessary right
now...)
>
> ```rust
> // Pseudo-Example
> pub trait BooleanVisitor:
> fn eval() -> boolean
> ```
+1 for splitting these into split modules. Instead of a boolean visitor, I'm
thinking about a post order predicate visitor:
```rust
pub trait BoundPredicateVisitor {
}
```
which is somehow motivated by @viirya 's
[pr](https://github.com/apache/iceberg-rust/pull/295/files#diff-a59622727cd67153abdf02031475bf8a1b1921738df4ca9903a685ff6970b7aaR472),
but move the travsering flow out of trait body.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
