alamb commented on code in PR #12014:
URL: https://github.com/apache/datafusion/pull/12014#discussion_r1719740786
##########
datafusion/sql/src/planner.rs:
##########
@@ -438,7 +438,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
}
SQLDataType::Bytea => Ok(DataType::Binary),
SQLDataType::Interval =>
Ok(DataType::Interval(IntervalUnit::MonthDayNano)),
- SQLDataType::Struct(fields) => {
+ SQLDataType::Struct(fields, _) => {
Review Comment:
Likewise here I think it would be good to name the field like
```suggestion
SQLDataType::Struct(fields, _bracket_type) => {
```
So it is easier to understand by reading just the code what is being ignored
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -178,7 +178,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
SQLExpr::Value(value) => {
self.parse_value(value,
planner_context.prepare_param_data_types())
}
- SQLExpr::Extract { field, expr } => {
+ SQLExpr::Extract { field, expr, .. } => {
Review Comment:
I recommend explicitly listing out the other fields here and returning a
NotImplemented error if there is some new syntax that DataFusion doesn't
support.
Otherwise what we have seen a few times in the past is that the parser
accepts some new syntax element but DataFusion just igores it silently
##########
datafusion/sql/src/utils.rs:
##########
@@ -268,6 +268,7 @@ pub(crate) fn value_to_string(value: &Value) ->
Option<String> {
Value::SingleQuotedString(s) => Some(s.to_string()),
Value::DollarQuotedString(s) => Some(s.to_string()),
Value::Number(_, _) | Value::Boolean(_) => Some(value.to_string()),
+ Value::UnicodeStringLiteral(s) => Some(s.to_string()),
Review Comment:
I think what you have here is ok
##########
datafusion/sqllogictest/test_files/expr.slt:
##########
@@ -848,8 +848,10 @@ SELECT EXTRACT("year" FROM timestamp
'2020-09-08T12:00:00+00:00')
----
2020
-query error
+query R
Review Comment:
🥳 🦜
##########
datafusion/sql/src/statement.rs:
##########
@@ -198,8 +198,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
match statement {
Statement::ExplainTable {
describe_alias: DescribeAlias::Describe, // only parse
'DESCRIBE table_name' and not 'EXPLAIN table_name'
- hive_format: _,
table_name,
+ ..
Review Comment:
per comment above, I recommend keeping the fields named explcitly
--
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
