waynexia commented on code in PR #15166:
URL: https://github.com/apache/datafusion/pull/15166#discussion_r1992048044


##########
datafusion/sql/src/statement.rs:
##########
@@ -214,11 +215,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
                 verbose,
                 statement,
                 analyze,
-                format: _,
+                format,
                 describe_alias: _,
                 ..
             } => {
-                self.explain_to_plan(verbose, analyze, 
DFStatement::Statement(statement))
+                let format = format.map(|format| format.to_string());

Review Comment:
   Not very related to this PR, will we change sqlparser part to use a `String` 
instead of enum like here? (I'd prefer so, as it gives us more flexibility)



##########
docs/source/user-guide/sql/explain.md:
##########
@@ -21,41 +21,219 @@
 
 The `EXPLAIN` command shows the logical and physical execution plan for the 
specified SQL statement.
 
-See the [Reading Explain Plans](../explain-usage.md) page for more information 
on how to interpret these plans.
+## Syntax
 
 <pre>
-EXPLAIN [ANALYZE] [VERBOSE] statement
+EXPLAIN [ANALYZE] [VERBOSE] [FORMAT format] statement

Review Comment:
   Maybe also state here that using `VERBOSE` and `FORMAT` together is not 
supported (yet?) like `ANALYZE` and `FORMAT` below



##########
datafusion/sql/src/parser.rs:
##########
@@ -43,12 +46,23 @@ fn parse_file_type(s: &str) -> Result<String, ParserError> {
     Ok(s.to_uppercase())
 }
 
-/// DataFusion specific EXPLAIN (needed so we can EXPLAIN datafusion
-/// specific COPY and other statements)
+/// DataFusion specific `EXPLAIN`
+///
+/// Syntax:
+/// ```sql
+/// EXPLAIN <ANALYZE> <VERBOSE> [FORMAT format] statement
+///```
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ExplainStatement {
+    /// `EXPLAIN ANALYZE ..`
     pub analyze: bool,
+    /// `EXPLAIN .. VERBOSE ..`
     pub verbose: bool,
+    /// `EXPLAIN .. FORMAT `
+    pub format: Option<String>,
+    /// The statement to analyze. Note this is a DataFusion [`Statement`] (not 
a
+    /// [`sqlparser::ast::Statement`] so that we can `EXPLAIN`  `COPY` and 
other

Review Comment:
   nit:
   ```suggestion
       /// [`sqlparser::ast::Statement`] so that we can use `EXPLAIN`, `COPY`, 
and other
   ```



-- 
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

Reply via email to