LucaCappelletti94 commented on code in PR #2425:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2425#discussion_r3718899365
##########
src/dialect/databricks.rs:
##########
Review Comment:
I believe there is some argument to be made that, since several of these
properties need to be kept aligned between Spark and Databricks, it may be
desirable to call directly Spark methods in these methods instead of
duplicating the scalar value.
##########
tests/sqlparser_databricks.rs:
##########
@@ -737,3 +737,55 @@ fn parse_cte_without_as() {
.parse_sql_statements("WITH cte (SELECT 1) SELECT * FROM cte")
.is_err());
}
+
+#[test]
+fn parse_create_table_using() {
+ match databricks().verified_stmt("CREATE TABLE t (id BIGINT) USING DELTA")
{
+ Statement::CreateTable(CreateTable { hive_formats, .. }) => {
+ assert_eq!(
+ hive_formats.unwrap().storage,
+ Some(HiveIOFormat::Using {
+ format: Ident::new("DELTA")
+ })
+ );
+ }
+ s => panic!("Unexpected statement: {s:?}"),
+ }
+
+ databricks().verified_stmt("CREATE TABLE IF NOT EXISTS t (id BIGINT) USING
PARQUET");
+
+ assert!(all_dialects_where(|d| !d.supports_create_table_using())
Review Comment:
Consider adding the analogous assert also for the other flags you have set
##########
tests/sqlparser_databricks.rs:
##########
@@ -737,3 +737,55 @@ fn parse_cte_without_as() {
.parse_sql_statements("WITH cte (SELECT 1) SELECT * FROM cte")
.is_err());
}
+
+#[test]
+fn parse_create_table_using() {
+ match databricks().verified_stmt("CREATE TABLE t (id BIGINT) USING DELTA")
{
+ Statement::CreateTable(CreateTable { hive_formats, .. }) => {
+ assert_eq!(
+ hive_formats.unwrap().storage,
+ Some(HiveIOFormat::Using {
+ format: Ident::new("DELTA")
+ })
+ );
+ }
+ s => panic!("Unexpected statement: {s:?}"),
+ }
+
+ databricks().verified_stmt("CREATE TABLE IF NOT EXISTS t (id BIGINT) USING
PARQUET");
+
+ assert!(all_dialects_where(|d| !d.supports_create_table_using())
Review Comment:
Also, I believe that since this assertion regards all dialects except
databricks, it should not be in the databricks tests, but in common.
##########
src/dialect/databricks.rs:
##########
@@ -113,4 +113,21 @@ impl Dialect for DatabricksDialect {
fn supports_select_item_multi_column_alias(&self) -> bool {
true
}
+
+ /// See
<https://docs.databricks.com/aws/en/sql/language-manual/sql-ref-syntax-ddl-create-table-using>
+ fn supports_create_table_using(&self) -> bool {
+ true
+ }
+
+ /// `LONG` is an alias for `BIGINT` in Databricks SQL.
+ ///
+ /// See
<https://docs.databricks.com/aws/en/sql/language-manual/data-types/bigint-type>
+ fn supports_long_type_as_bigint(&self) -> bool {
+ true
+ }
+
Review Comment:
While you are adding support for Spark flags in Databricks, I believe you
should also add `supports_pipe_operator` and `parse_infix` for `DIV`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
