findepi commented on code in PR #12793:
URL: https://github.com/apache/datafusion/pull/12793#discussion_r1793256984
##########
datafusion/expr-common/src/scalar.rs:
##########
@@ -76,6 +79,30 @@ impl PartialEq for Scalar {
}
}
+impl PartialOrd for Scalar {
+ fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+ self.value.partial_cmp(&other.value)
+ }
+}
+
+impl Hash for Scalar {
+ fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+ self.value.hash(state);
+ }
+}
+
+impl fmt::Display for Scalar {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ self.value.fmt(f)
+ }
+}
+
+impl fmt::Debug for Scalar {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ self.value.fmt(f)
Review Comment:
Please keep derived Debug. It reports both value & type, which might be
useful for debugging.
##########
datafusion/expr-common/src/scalar.rs:
##########
@@ -116,6 +143,10 @@ impl Scalar {
ScalarValue::iter_to_array_of_type(scalars.map(|scalar| scalar.value),
&data_type)
}
+ pub fn cast_to(&self, data_type: &DataType) -> Result<Self> {
Review Comment:
Do we need this on the Scalar itself?
Once we logicalize it, this function will drag us. Maybe let's have a utlity
method somewhere else.
##########
datafusion/expr/src/literal.rs:
##########
@@ -41,39 +42,45 @@ pub trait TimestampLiteral {
fn lit_timestamp_nano(&self) -> Expr;
}
+impl From<ScalarValue> for Expr {
+ fn from(value: ScalarValue) -> Self {
+ Self::Literal(Scalar::from(value))
Review Comment:
at some point we should rename Literal to Constant. constant is what this
represents, while literal is a syntax-related term.
##########
datafusion/expr/src/expr_rewriter/mod.rs:
##########
@@ -380,14 +380,13 @@ mod test {
// rewrites all "foo" string literals to "bar"
let transformer = |expr: Expr| -> Result<Transformed<Expr>> {
match expr {
- Expr::Literal(ScalarValue::Utf8(Some(utf8_val))) => {
- let utf8_val = if utf8_val == "foo" {
- "bar".to_string()
- } else {
- utf8_val
- };
Review Comment:
nit: avoid formatting changes unless needed
--
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]
