sdd commented on code in PR #169:
URL: https://github.com/apache/iceberg-rust/pull/169#discussion_r1498011642
##########
crates/iceberg/src/expr/mod.rs:
##########
@@ -18,12 +18,15 @@
//! This module contains expressions.
mod term;
+
+use std::fmt::{Display, Formatter};
pub use term::*;
mod predicate;
pub use predicate::*;
/// Predicate operators used in expressions.
#[allow(missing_docs)]
+#[derive(Debug, Clone, Copy)]
pub enum PredicateOperator {
Review Comment:
Would it be useful here to use the discriminant to encode if the operator is
unary or binary? E.g.
```rust
pub enum PredicateOperator {
// Unary operators
IsNull = 100,
NotNull,
IsNan,
NotNan,
// Binary operators
LessThan = 200,
LessThanOrEq,
GreaterThan,
GreaterThanOrEq,
Eq,
NotEq,
In,
NotIn,
StartsWith,
NotStartsWith,
}
```
Then when constructing e.g. a `UnaryExpression` you can at least assert on
this to give safety at runtime in debug mode:
```rust
impl<T: Debug> UnaryExpression<T> {
pub(crate) fn new(op: PredicateOperator, term: T) -> Self {
debug_assert!(op as u32 < 200);
Self { op, term }
}
}
// ...
impl<T: Debug> BinaryExpression<T> {
pub(crate) fn new(op: PredicateOperator, term: T, literal: Datum) ->
Self {
debug_assert!(op as u32 >= 200);
Self { op, term, literal }
}
}
```
--
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]
