iffyio commented on code in PR #1633:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1633#discussion_r1903254863
##########
src/ast/mod.rs:
##########
@@ -7698,6 +7698,25 @@ impl fmt::Display for RenameTable {
}
}
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub enum TableObject {
Review Comment:
Could we include a doc comment mentionign this is the target of an insert
table statement? And then for the variants that it can either be a tablename or
function with example SQL? That would make it easy for folks to get an overview
of the struct without having to dig into the code or test out manually
##########
src/parser/mod.rs:
##########
@@ -8857,6 +8857,23 @@ impl<'a> Parser<'a> {
}
}
+ pub fn parse_table_object(
+ &mut self,
+ in_table_clause: bool,
+ ) -> Result<TableObject, ParserError> {
+ if dialect_of!(self is ClickHouseDialect) &&
self.parse_keyword(Keyword::FUNCTION) {
Review Comment:
Can we use a dialect method for this? e.g.
```rust
if self.dialect.supports_function_table_objects() &&
self.parse_keyword(keyword::FUNCTION)
```
##########
src/ast/dml.rs:
##########
@@ -470,8 +470,7 @@ pub struct Insert {
/// INTO - optional keyword
pub into: bool,
/// TABLE
- #[cfg_attr(feature = "visitor", visit(with = "visit_relation"))]
- pub table_name: ObjectName,
+ pub table_object: TableObject,
Review Comment:
```suggestion
pub table: TableObject,
```
##########
tests/sqlparser_clickhouse.rs:
##########
@@ -222,6 +222,11 @@ fn parse_create_table() {
);
}
+#[test]
+fn parse_insert_into_function() {
+ clickhouse().verified_stmt(r#"INSERT INTO TABLE FUNCTION
remote('localhost', default.simple_table) VALUES (100, 'inserted via
remote()')"#);
Review Comment:
Can we add a similar test case without the `TABLE` keyword?
##########
src/parser/mod.rs:
##########
@@ -8857,6 +8857,23 @@ impl<'a> Parser<'a> {
}
}
+ pub fn parse_table_object(
Review Comment:
Can we add a doc comment for this given its a public method?
##########
src/parser/mod.rs:
##########
@@ -8857,6 +8857,23 @@ impl<'a> Parser<'a> {
}
}
+ pub fn parse_table_object(
+ &mut self,
+ in_table_clause: bool,
Review Comment:
Is the `in_table_clause` flag needed? I imagine it'll always be false
##########
src/parser/mod.rs:
##########
@@ -8857,6 +8857,23 @@ impl<'a> Parser<'a> {
}
}
+ pub fn parse_table_object(
+ &mut self,
+ in_table_clause: bool,
+ ) -> Result<TableObject, ParserError> {
+ if dialect_of!(self is ClickHouseDialect) &&
self.parse_keyword(Keyword::FUNCTION) {
+ self.parse_table_function().map(TableObject::TableFunction)
+ } else {
+ self.parse_object_name(in_table_clause)
+ .map(TableObject::TableName)
+ }
+ }
+
+ pub fn parse_table_function(&mut self) -> Result<Function, ParserError> {
+ let fn_name = self.parse_object_name(false)?;
+ self.parse_function_call(fn_name)
+ }
Review Comment:
I'm wondering if its worth having this as a standalone method? It looks like
it can be inlined given its only 2 LOC?
--
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
