iffyio commented on code in PR #2013:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/2013#discussion_r2306975457


##########
src/ast/mod.rs:
##########
@@ -8796,16 +8831,46 @@ pub enum CopyLegacyOption {
     Delimiter(char),
     /// EMPTYASNULL
     EmptyAsNull,
+    /// ENCRYPTED \[ AUTO \]
+    Encrypted { auto: bool },
+    /// ESCAPE
+    Escape,
+    /// EXTENSION 'extension-name'
+    Extension(String),
+    /// FIXEDWIDTH \[ AS \] 'fixedwidth-spec'
+    FixedWidth(String),
+    /// GZIP
+    Gzip,
+    /// HEADER
+    Header,
     /// IAM_ROLE { DEFAULT | 'arn:aws:iam::123456789:role/role1' }
     IamRole(IamRoleKind),
     /// IGNOREHEADER \[ AS \] number_rows
     IgnoreHeader(u64),
+    /// JSON
+    Json,
+    /// MANIFEST \[ VERBOSE \]
+    Manifest { verbose: bool },
+    /// MAXFILESIZE \[ AS \] max-size \[ MB | GB \]
+    MaxFileSize(FileSize),
     /// NULL \[ AS \] 'null_string'
     Null(String),
+    /// PARALLEL
+    Parallel(Option<bool>),

Review Comment:
   can we add a description here to mention the behavior of on/off?



##########
src/ast/mod.rs:
##########
@@ -8849,10 +8946,73 @@ impl fmt::Display for CopyLegacyOption {
                 Ok(())
             }
             TruncateColumns => write!(f, "TRUNCATECOLUMNS"),
+            Zstd => write!(f, "ZSTD"),
+        }
+    }
+}
+
+/// ```sql
+/// SIZE \[ MB | GB \]
+/// ```
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub struct FileSize {
+    pub size: Value,
+    pub unit: Option<FileSizeUnit>,
+}
+
+impl fmt::Display for FileSize {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.size)?;
+        if let Some(unit) = &self.unit {
+            write!(f, " {unit}")?;
+        }
+        Ok(())
+    }
+}
+
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub enum FileSizeUnit {
+    MB,
+    GB,
+}
+
+impl fmt::Display for FileSizeUnit {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            FileSizeUnit::MB => write!(f, "MB"),
+            FileSizeUnit::GB => write!(f, "GB"),
         }
     }
 }
 
+/// Specifies the partition keys for the unload operation
+///
+/// ```sql
+/// PARTITION BY ( column_name [, ... ] ) [ INCLUDE ]
+/// ```
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub struct PartitionBy {

Review Comment:
   ```suggestion
   pub struct UnloadPartitionBy {
   ```
   Thinking since partitionby has meaning in other contexts?



##########
src/parser/mod.rs:
##########
@@ -9657,16 +9683,81 @@ impl<'a> Parser<'a> {
                 CopyLegacyOption::Delimiter(self.parse_literal_char()?)
             }
             Some(Keyword::EMPTYASNULL) => CopyLegacyOption::EmptyAsNull,
+            Some(Keyword::ENCRYPTED) => {
+                let auto = self.parse_keyword(Keyword::AUTO);
+                CopyLegacyOption::Encrypted { auto }
+            }
+            Some(Keyword::ESCAPE) => CopyLegacyOption::Escape,
+            Some(Keyword::EXTENSION) => {
+                let ext = self.parse_literal_string()?;
+                CopyLegacyOption::Extension(ext)
+            }
+            Some(Keyword::FIXEDWIDTH) => {
+                let spec = self.parse_literal_string()?;
+                CopyLegacyOption::FixedWidth(spec)
+            }
+            Some(Keyword::GZIP) => CopyLegacyOption::Gzip,
+            Some(Keyword::HEADER) => CopyLegacyOption::Header,
             Some(Keyword::IAM_ROLE) => 
CopyLegacyOption::IamRole(self.parse_iam_role_kind()?),
             Some(Keyword::IGNOREHEADER) => {
                 let _ = self.parse_keyword(Keyword::AS);
                 let num_rows = self.parse_literal_uint()?;
                 CopyLegacyOption::IgnoreHeader(num_rows)
             }
+            Some(Keyword::JSON) => CopyLegacyOption::Json,
+            Some(Keyword::MANIFEST) => {
+                let verbose = self.parse_keyword(Keyword::VERBOSE);
+                CopyLegacyOption::Manifest { verbose }
+            }
+            Some(Keyword::MAXFILESIZE) => {
+                let _ = self.parse_keyword(Keyword::AS);
+                let size = self.parse_number_value()?.value;
+                let unit = match self.parse_one_of_keywords(&[Keyword::MB, 
Keyword::GB]) {
+                    Some(Keyword::MB) => Some(FileSizeUnit::MB),
+                    Some(Keyword::GB) => Some(FileSizeUnit::GB),
+                    _ => None,
+                };
+                CopyLegacyOption::MaxFileSize(FileSize { size, unit })
+            }
             Some(Keyword::NULL) => {
                 let _ = self.parse_keyword(Keyword::AS);
                 CopyLegacyOption::Null(self.parse_literal_string()?)
             }
+            Some(Keyword::PARALLEL) => {
+                let enabled = match self.parse_one_of_keywords(&[
+                    Keyword::TRUE,
+                    Keyword::FALSE,
+                    Keyword::ON,
+                    Keyword::OFF,
+                ]) {
+                    Some(Keyword::TRUE) | Some(Keyword::ON) => Some(true),
+                    Some(Keyword::FALSE) | Some(Keyword::OFF) => Some(false),
+                    _ => None,
+                };
+                CopyLegacyOption::Parallel(enabled)
+            }
+            Some(Keyword::PARQUET) => CopyLegacyOption::Parquet,
+            Some(Keyword::PARTITION) => {
+                self.expect_keyword(Keyword::BY)?;
+                let columns = 
self.parse_parenthesized_column_list(IsOptional::Mandatory, false)?;
+                let include = self.parse_keyword(Keyword::INCLUDE);
+                CopyLegacyOption::PartitionBy(PartitionBy { columns, include })
+            }
+            Some(Keyword::REGION) => {
+                let _ = self.parse_keyword(Keyword::AS);
+                let region = self.parse_literal_string()?;
+                CopyLegacyOption::Region(region)
+            }
+            Some(Keyword::ROWGROUPSIZE) => {
+                let _ = self.parse_keyword(Keyword::AS);
+                let size = self.parse_number_value()?.value;
+                let unit = match self.parse_one_of_keywords(&[Keyword::MB, 
Keyword::GB]) {
+                    Some(Keyword::MB) => Some(FileSizeUnit::MB),
+                    Some(Keyword::GB) => Some(FileSizeUnit::GB),
+                    _ => None,
+                };
+                CopyLegacyOption::RowGroupSize(FileSize { size, unit })

Review Comment:
   Can we pull this out to a function like `parse_file_size_unit()` so that it 
can be reused?



##########
src/parser/mod.rs:
##########
@@ -16477,19 +16569,35 @@ impl<'a> Parser<'a> {
     }
 
     pub fn parse_unload(&mut self) -> Result<Statement, ParserError> {
+        self.expect_keyword(Keyword::UNLOAD)?;
         self.expect_token(&Token::LParen)?;
-        let query = self.parse_query()?;
+        let (query, query_text) = if matches!(self.peek_token().token, 
Token::SingleQuotedString(_))
+        {
+            (None, Some(self.parse_literal_string()?))
+        } else {
+            (Some(self.parse_query()?), None)
+        };
         self.expect_token(&Token::RParen)?;
 
         self.expect_keyword_is(Keyword::TO)?;
         let to = self.parse_identifier()?;
-
-        let with_options = self.parse_options(Keyword::WITH)?;
-
+        let auth = if self.parse_keyword(Keyword::IAM_ROLE) {
+            Some(self.parse_iam_role_kind()?)
+        } else {
+            None
+        };
+        let with = self.parse_options(Keyword::WITH)?;

Review Comment:
   Can we add a test case that uses the `WITH` keyword in the syntax?



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