alamb commented on code in PR #15163:
URL: https://github.com/apache/datafusion/pull/15163#discussion_r1993875533
##########
datafusion/physical-expr-common/src/physical_expr.rs:
##########
@@ -266,6 +266,8 @@ pub trait PhysicalExpr: Send + Sync + Display + Debug +
DynEq + DynHash {
fn get_properties(&self, _children: &[ExprProperties]) ->
Result<ExprProperties> {
Ok(ExprProperties::new_unknown())
}
+
+ fn fmt_sql(&self, f: &mut Formatter<'_>) -> std::fmt::Result;
Review Comment:
I think we should add some documentation. Something like the following
```suggestion
/// Format this `PhysicalExpr` in nice human readable "SQL" format
///
/// Specifically, this format is designed to be readable by humans, at
the
/// expense of details. Use `Display` or `Debug` for more detailed
/// representation.
///
fn fmt_sql(&self, f: &mut Formatter<'_>) -> std::fmt::Result;
```
Also I think we should add some guidance in the overall documentation of
`PhysicalExpr` about the differences between `Debug`, `Display` and `fmt_sql`?
SOmething like the following perhaps:
```rust
///
/// # Formatting `PhysicalExpr` as strings
/// There are three ways to format `PhysicalExpr` as a string:
/// * [`Debug`]: Standard Rust debugging format (e.g. `Constant { value: ...
}`)
/// * [`Display`]: Detailed SQL-like format that shows expression structure
(e.g. (`Utf8 ("foobar")`). This is often used for debugging and tests
/// *[`Self::fmt_sql`]: SQL-like human readable format (e.g. ('foobar')`)
///
```
```
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
