alamb commented on code in PR #15249:
URL: https://github.com/apache/datafusion/pull/15249#discussion_r1997551623
##########
datafusion/datasource-parquet/src/source.rs:
##########
@@ -580,7 +581,7 @@ impl FileSource for ParquetSource {
}
DisplayFormatType::TreeRender => {
if let Some(predicate) = self.predicate() {
- writeln!(f, "predicate={predicate}")?;
+ writeln!(f, "predicate={}", fmt_sql(predicate.as_ref()))?;
Review Comment:
❤️
##########
datafusion/sqllogictest/test_files/explain_tree.slt:
##########
@@ -175,7 +175,7 @@ physical_plan
08)│ FilterExec │
09)│ -------------------- │
10)│ predicate: │
-11)│ string_col@1 != foo │
+11)│ string_col != foo │
Review Comment:
so nice
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -830,6 +830,7 @@ impl DisplayAs for AggregateExec {
})
.collect()
};
+ // TODO: Implement `fmt_sql` for `AggregateFunctionExpr`.
Review Comment:
perhaps we can make a follow on ticket for this
##########
datafusion/sqllogictest/test_files/explain_tree.slt:
##########
@@ -1569,20 +1570,20 @@ physical_plan
01)┌───────────────────────────┐
02)│ SortPreservingMergeExec │
03)│ -------------------- │
-04)│ date@0 ASC NULLS LAST, │
-05)│ time@2 ASC NULLS LAST │
-06)└─────────────┬─────────────┘
-07)┌─────────────┴─────────────┐
-08)│ CoalesceBatchesExec │
-09)│ -------------------- │
-10)│ target_batch_size: │
-11)│ 8192 │
-12)└─────────────┬─────────────┘
-13)┌─────────────┴─────────────┐
-14)│ FilterExec │
-15)│ -------------------- │
-16)│ predicate: │
-17)│ ticker@1 = A │
+04)│ date ASC NULLS LAST, │
+05)│ │
Review Comment:
it is somewhat strange to me there is an extra newline. Is that something we
can remove?
##########
datafusion/physical-expr-common/src/sort_expr.rs:
##########
@@ -117,6 +117,16 @@ impl PhysicalSortExpr {
self.options.nulls_first = false;
self
}
+
+ /// Like `fmt_sql` in `PhysicalExpr`, prints a [`PhysicalSortExpr`] in a
SQL-like format.
Review Comment:
If you put the whole thing in `[]` then rustdoc will make an automatic link
```suggestion
/// Like [`PhysicalExpr::fmt_sql`] prints a [`PhysicalSortExpr`] in a
SQL-like format.
```
##########
datafusion/sqllogictest/test_files/explain_tree.slt:
##########
@@ -1733,21 +1734,67 @@ physical_plan
01)┌───────────────────────────┐
02)│ SortPreservingMergeExec │
03)│ -------------------- │
-04)│ time@2 ASC NULLS LAST, │
-05)│ date@0 ASC NULLS LAST │
-06)└─────────────┬─────────────┘
-07)┌─────────────┴─────────────┐
-08)│ CoalesceBatchesExec │
-09)│ -------------------- │
-10)│ target_batch_size: │
-11)│ 8192 │
-12)└─────────────┬─────────────┘
-13)┌─────────────┴─────────────┐
-14)│ FilterExec │
-15)│ -------------------- │
-16)│ predicate: │
-17)│ ticker@1 = A AND CAST(time│
-18)│ @2 AS Date32) = date@0 │
+04)│ date ASC NULLS LAST │
+05)│ │
+06)│ time ASC NULLS LAST, │
+07)└─────────────┬─────────────┘
+08)┌─────────────┴─────────────┐
+09)│ CoalesceBatchesExec │
+10)│ -------------------- │
+11)│ target_batch_size: │
+12)│ 8192 │
+13)└─────────────┬─────────────┘
+14)┌─────────────┴─────────────┐
+15)│ FilterExec │
+16)│ -------------------- │
+17)│ predicate: │
+18)│ ticker = A AND CAST(time │
Review Comment:
this looks so nice
(I am using that image a log recently 🤔 )
--
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
