You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "aprimadi (via GitHub)" <gi...@apache.org> on 2023/05/15 11:18:15 UTC

[GitHub] [arrow-datafusion] aprimadi opened a new pull request, #6352: Refactor create external table to use one_of_keywords

aprimadi opened a new pull request, #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352

   # Which issue does this PR close?
   
   Closes #6251
   
   # Rationale for this change
   
   N/A
   
   # What changes are included in this PR?
   
   - Refactor create external table parser to use one_of_keywords
   - 
   
   # Are these changes tested?
   
   TODO
   
   # Are there any user-facing changes?
   
   No API change. Add more SQL functionality.


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] mustafasrepo merged pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "mustafasrepo (via GitHub)" <gi...@apache.org>.
mustafasrepo merged PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] mustafasrepo commented on pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "mustafasrepo (via GitHub)" <gi...@apache.org>.
mustafasrepo commented on PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352#issuecomment-1549204696

   This PR is LGTM!. Thanks @aprimadi for this work.


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] aprimadi commented on pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "aprimadi (via GitHub)" <gi...@apache.org>.
aprimadi commented on PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352#issuecomment-1550013344

   Thank you for the review @mustafasrepo 


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] mustafasrepo commented on a diff in pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "mustafasrepo (via GitHub)" <gi...@apache.org>.
mustafasrepo commented on code in PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352#discussion_r1194783100


##########
datafusion/sql/src/parser.rs:
##########
@@ -427,39 +435,72 @@ impl<'a> DFParser<'a> {
         }
 
         loop {
-            if self.parser.parse_keyword(Keyword::STORED) {
-                self.parser.expect_keyword(Keyword::AS)?;
-                ensure_not_set(&builder.file_type, "STORED AS")?;
-                builder.file_type = Some(self.parse_file_format()?);
-            } else if self.parser.parse_keyword(Keyword::LOCATION) {
-                ensure_not_set(&builder.location, "LOCATION")?;
-                builder.location = Some(self.parser.parse_literal_string()?);
-            } else if self.parser.parse_keyword(Keyword::WITH) {
-                if self.parser.parse_keyword(Keyword::ORDER) {
-                    ensure_not_set(&builder.order_exprs, "WITH ORDER")?;
-                    builder.order_exprs = Some(self.parse_order_by_exprs()?);
-                } else {
-                    self.parser.expect_keyword(Keyword::HEADER)?;
-                    self.parser.expect_keyword(Keyword::ROW)?;
-                    ensure_not_set(&builder.has_header, "WITH HEADER ROW")?;
-                    builder.has_header = Some(true);
+            if let Some(keyword) = self.parser.parse_one_of_keywords(&[

Review Comment:
   Nice cleanup!.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] alamb commented on pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352#issuecomment-1550365330

   🤔  github seems to be acting up (not merging this PR). 


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] alamb commented on pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352#issuecomment-1550365037

   Thanks @aprimadi  and @mustafasrepo !


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] aprimadi commented on pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "aprimadi (via GitHub)" <gi...@apache.org>.
aprimadi commented on PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352#issuecomment-1550652378

   @alamb is it supposed to merge automatically?


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] aprimadi commented on a diff in pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "aprimadi (via GitHub)" <gi...@apache.org>.
aprimadi commented on code in PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352#discussion_r1194778613


##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -192,6 +192,8 @@ pub struct CreateExternalTable {
     pub order_exprs: Vec<Expr>,
     /// File compression type (GZIP, BZIP2, XZ, ZSTD)
     pub file_compression_type: CompressionTypeVariant,
+    /// Whether the table is an infinite streams
+    pub unbounded: bool,

Review Comment:
   Ah yes, I miss that



##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -192,6 +192,8 @@ pub struct CreateExternalTable {
     pub order_exprs: Vec<Expr>,
     /// File compression type (GZIP, BZIP2, XZ, ZSTD)
     pub file_compression_type: CompressionTypeVariant,
+    /// Whether the table is an infinite streams
+    pub unbounded: bool,

Review Comment:
   Thank you



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] mustafasrepo commented on a diff in pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "mustafasrepo (via GitHub)" <gi...@apache.org>.
mustafasrepo commented on code in PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352#discussion_r1194774776


##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -192,6 +192,8 @@ pub struct CreateExternalTable {
     pub order_exprs: Vec<Expr>,
     /// File compression type (GZIP, BZIP2, XZ, ZSTD)
     pub file_compression_type: CompressionTypeVariant,
+    /// Whether the table is an infinite streams
+    pub unbounded: bool,

Review Comment:
   I think, `fn hash<H: Hasher>(&self, state: &mut H)` function should use `self.unbounded` during hashing also.



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow-datafusion] alamb commented on pull request #6352: Support CREATE TABLE via SQL for infinite streams

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6352:
URL: https://github.com/apache/arrow-datafusion/pull/6352#issuecomment-1548459907

   cc @ozankabak and @mustafasrepo 


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org