You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "funcpp (via GitHub)" <gi...@apache.org> on 2026/01/22 06:01:34 UTC
[PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
funcpp opened a new pull request, #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170
## Summary
This PR adds several Databricks Delta Lake SQL syntax features:
### 1. OPTIMIZE statement support
Adds support for the Databricks `OPTIMIZE` statement syntax:
```sql
OPTIMIZE table_name [WHERE predicate] [ZORDER BY (col1, col2, ...)]
```
Reference: https://docs.databricks.com/en/sql/language-manual/delta-optimize.html
Key difference from ClickHouse: Databricks omits the `TABLE` keyword after `OPTIMIZE`.
### 2. PARTITIONED BY with optional column types
Databricks allows partition columns to reference existing table columns without specifying types:
```sql
CREATE TABLE t (col1 STRING, col2 INT) PARTITIONED BY (col1)
CREATE TABLE t (name STRING) PARTITIONED BY (year INT, month INT)
```
Reference: https://docs.databricks.com/en/sql/language-manual/sql-ref-partition.html
### 3. STRUCT type with colon syntax
Databricks uses Hive-style colon separator for struct field definitions:
```sql
STRUCT<field_name: field_type, ...>
ARRAY<STRUCT<finish_flag: STRING, survive_flag: STRING, score: INT>>
```
Reference: https://docs.databricks.com/en/sql/language-manual/data-types/struct-type.html
The colon is optional per the spec, so both `field: type` and `field type` syntaxes are now accepted.
## Changes
- Extended `OptimizeTable` AST to support Databricks-specific fields (`predicate`, `zorder`, `has_table_keyword`)
- Added `parse_column_def_for_partition()` to handle optional column types in PARTITIONED BY
- Added `DatabricksDialect` to STRUCT type parsing
- Modified `parse_struct_field_def()` to accept optional colon separator
## Test plan
- [x] Added tests for `OPTIMIZE` statement variations
- [x] Added tests for `PARTITIONED BY` with/without column types
- [x] Added tests for `STRUCT` type with colon syntax
- [x] Verified existing ClickHouse and BigQuery tests still pass
- [x] All tests pass (`cargo test`)
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "funcpp (via GitHub)" <gi...@apache.org>.
funcpp commented on code in PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#discussion_r2772342945
##########
src/parser/mod.rs:
##########
@@ -7907,14 +7890,41 @@ impl<'a> Parser<'a> {
pub fn parse_hive_distribution(&mut self) -> Result<HiveDistributionStyle, ParserError> {
if self.parse_keywords(&[Keyword::PARTITIONED, Keyword::BY]) {
self.expect_token(&Token::LParen)?;
- let columns = self.parse_comma_separated(Parser::parse_column_def)?;
+ let columns = self.parse_comma_separated(Parser::parse_column_def_for_partition)?;
self.expect_token(&Token::RParen)?;
Ok(HiveDistributionStyle::PARTITIONED { columns })
} else {
Ok(HiveDistributionStyle::NONE)
}
}
+ /// Parse column definition for PARTITIONED BY clause.
+ ///
+ /// Databricks allows partition columns without types when referencing
+ /// columns already defined in the table specification:
+ /// ```sql
+ /// CREATE TABLE t (col1 STRING, col2 INT) PARTITIONED BY (col1)
+ /// CREATE TABLE t (col1 STRING) PARTITIONED BY (col2 INT)
+ /// ```
+ ///
+ /// See [Databricks](https://docs.databricks.com/en/sql/language-manual/sql-ref-partition.html)
+ fn parse_column_def_for_partition(&mut self) -> Result<ColumnDef, ParserError> {
Review Comment:
@iffyio could you please review? 👀
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "funcpp (via GitHub)" <gi...@apache.org>.
funcpp commented on PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#issuecomment-3867106793
> @funcpp could you take a look when you have some time to resolve the conflicts on the branch?
@iffyio okay. I will resolve these conflicts tomorrow!
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for `OPTIMIZE`, `PARTITIONED BY`, and `STRUCT` [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio merged PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#issuecomment-3787629155
LGTM @funcpp, but could you fix the clippy issue
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "funcpp (via GitHub)" <gi...@apache.org>.
funcpp commented on code in PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#discussion_r2761955818
##########
src/parser/mod.rs:
##########
@@ -7907,14 +7890,41 @@ impl<'a> Parser<'a> {
pub fn parse_hive_distribution(&mut self) -> Result<HiveDistributionStyle, ParserError> {
if self.parse_keywords(&[Keyword::PARTITIONED, Keyword::BY]) {
self.expect_token(&Token::LParen)?;
- let columns = self.parse_comma_separated(Parser::parse_column_def)?;
+ let columns = self.parse_comma_separated(Parser::parse_column_def_for_partition)?;
self.expect_token(&Token::RParen)?;
Ok(HiveDistributionStyle::PARTITIONED { columns })
} else {
Ok(HiveDistributionStyle::NONE)
}
}
+ /// Parse column definition for PARTITIONED BY clause.
+ ///
+ /// Databricks allows partition columns without types when referencing
+ /// columns already defined in the table specification:
+ /// ```sql
+ /// CREATE TABLE t (col1 STRING, col2 INT) PARTITIONED BY (col1)
+ /// CREATE TABLE t (col1 STRING) PARTITIONED BY (col2 INT)
+ /// ```
+ ///
+ /// See [Databricks](https://docs.databricks.com/en/sql/language-manual/sql-ref-partition.html)
+ fn parse_column_def_for_partition(&mut self) -> Result<ColumnDef, ParserError> {
Review Comment:
Makes sense\! I've extracted the shared logic into `parse_column_def_inner(optional_data_type: bool)` so that `parse_column_def` delegates to it with `false` (type required) and the PARTITIONED BY path calls it with `true` (type optional via `maybe_parse`). This removes the duplicate function while reusing the full column def parsing logic including options.
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#discussion_r2741645683
##########
src/parser/mod.rs:
##########
@@ -7907,14 +7890,41 @@ impl<'a> Parser<'a> {
pub fn parse_hive_distribution(&mut self) -> Result<HiveDistributionStyle, ParserError> {
if self.parse_keywords(&[Keyword::PARTITIONED, Keyword::BY]) {
self.expect_token(&Token::LParen)?;
- let columns = self.parse_comma_separated(Parser::parse_column_def)?;
+ let columns = self.parse_comma_separated(Parser::parse_column_def_for_partition)?;
self.expect_token(&Token::RParen)?;
Ok(HiveDistributionStyle::PARTITIONED { columns })
} else {
Ok(HiveDistributionStyle::NONE)
}
}
+ /// Parse column definition for PARTITIONED BY clause.
+ ///
+ /// Databricks allows partition columns without types when referencing
+ /// columns already defined in the table specification:
+ /// ```sql
+ /// CREATE TABLE t (col1 STRING, col2 INT) PARTITIONED BY (col1)
+ /// CREATE TABLE t (col1 STRING) PARTITIONED BY (col2 INT)
+ /// ```
+ ///
+ /// See [Databricks](https://docs.databricks.com/en/sql/language-manual/sql-ref-partition.html)
+ fn parse_column_def_for_partition(&mut self) -> Result<ColumnDef, ParserError> {
Review Comment:
Oh I see they're indeed the same cases. It would seem that `ColumnDef` should have an `Option`al datatype instead of us introducing a special datatype but that issue is unrelated to this PR.
For this PR though this new function seems to duplicate `parse_column_def` (minus the aforementioned optional datatype case) so that it would be good to reuse it instead. I can imagine something like?
```rust
pub fn parse_column_def(&mut self) -> Result<ColumnDef, ParserError> {
parse_column_def_inner(true)
}
fn parse_column_def_inner(&mut self) -> Result<ColumnDef, ParserError> {
// ...
let data_type = if self.is_column_type_sqlite_unspecified() {
DataType::Unspecified
} else {
self
.maybe_parse(|parser| parser.parse_data_type())?
.unwrap_or(DataType::Unspecified)
};
// ...
}
```
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "funcpp (via GitHub)" <gi...@apache.org>.
funcpp commented on code in PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#discussion_r2726119591
##########
src/parser/mod.rs:
##########
@@ -7907,14 +7890,41 @@ impl<'a> Parser<'a> {
pub fn parse_hive_distribution(&mut self) -> Result<HiveDistributionStyle, ParserError> {
if self.parse_keywords(&[Keyword::PARTITIONED, Keyword::BY]) {
self.expect_token(&Token::LParen)?;
- let columns = self.parse_comma_separated(Parser::parse_column_def)?;
+ let columns = self.parse_comma_separated(Parser::parse_column_def_for_partition)?;
self.expect_token(&Token::RParen)?;
Ok(HiveDistributionStyle::PARTITIONED { columns })
} else {
Ok(HiveDistributionStyle::NONE)
}
}
+ /// Parse column definition for PARTITIONED BY clause.
+ ///
+ /// Databricks allows partition columns without types when referencing
+ /// columns already defined in the table specification:
+ /// ```sql
+ /// CREATE TABLE t (col1 STRING, col2 INT) PARTITIONED BY (col1)
+ /// CREATE TABLE t (col1 STRING) PARTITIONED BY (col2 INT)
+ /// ```
+ ///
+ /// See [Databricks](https://docs.databricks.com/en/sql/language-manual/sql-ref-partition.html)
+ fn parse_column_def_for_partition(&mut self) -> Result<ColumnDef, ParserError> {
Review Comment:
Since `DataType::Unspecified` already exists and is used by SQLite for similar "type-less" scenarios, I thought reusing it would keep things simpler without adding another AST type. Do two cases have different meaning?
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "funcpp (via GitHub)" <gi...@apache.org>.
funcpp commented on PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#issuecomment-3787653895
> LGTM @funcpp, but could you fix the clippy issue
@andygrove sure, I fixed it.
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#discussion_r2724067066
##########
src/parser/mod.rs:
##########
@@ -18233,13 +18244,24 @@ impl<'a> Parser<'a> {
}
}
+ /// ClickHouse:
/// ```sql
/// OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition | PARTITION ID 'partition_id'] [FINAL] [DEDUPLICATE [BY expression]]
/// ```
/// [ClickHouse](https://clickhouse.com/docs/en/sql-reference/statements/optimize)
+ ///
+ /// Databricks:
+ /// ```sql
+ /// OPTIMIZE table_name [WHERE predicate] [ZORDER BY (col_name1 [, ...])]
+ /// ```
+ /// [Databricks](https://docs.databricks.com/en/sql/language-manual/delta-optimize.html)
pub fn parse_optimize_table(&mut self) -> Result<Statement, ParserError> {
- self.expect_keyword_is(Keyword::TABLE)?;
+ // Check for TABLE keyword (ClickHouse uses it, Databricks does not)
Review Comment:
```suggestion
```
thinking we can skip the comment so that it doesnt become stale/incomplete if other dialects support the feature
##########
src/parser/mod.rs:
##########
@@ -7907,14 +7890,41 @@ impl<'a> Parser<'a> {
pub fn parse_hive_distribution(&mut self) -> Result<HiveDistributionStyle, ParserError> {
if self.parse_keywords(&[Keyword::PARTITIONED, Keyword::BY]) {
self.expect_token(&Token::LParen)?;
- let columns = self.parse_comma_separated(Parser::parse_column_def)?;
+ let columns = self.parse_comma_separated(Parser::parse_column_def_for_partition)?;
self.expect_token(&Token::RParen)?;
Ok(HiveDistributionStyle::PARTITIONED { columns })
} else {
Ok(HiveDistributionStyle::NONE)
}
}
+ /// Parse column definition for PARTITIONED BY clause.
+ ///
+ /// Databricks allows partition columns without types when referencing
+ /// columns already defined in the table specification:
+ /// ```sql
+ /// CREATE TABLE t (col1 STRING, col2 INT) PARTITIONED BY (col1)
+ /// CREATE TABLE t (col1 STRING) PARTITIONED BY (col2 INT)
+ /// ```
+ ///
+ /// See [Databricks](https://docs.databricks.com/en/sql/language-manual/sql-ref-partition.html)
+ fn parse_column_def_for_partition(&mut self) -> Result<ColumnDef, ParserError> {
Review Comment:
```suggestion
fn parse_partitioned_by_column_def(&mut self) -> Result<ColumnDef, ParserError> {
```
##########
src/parser/mod.rs:
##########
@@ -3361,25 +3334,35 @@ impl<'a> Parser<'a> {
/// Syntax:
///
/// ```sql
+ /// -- BigQuery style
/// [field_name] field_type
+ /// -- Databricks/Hive style (colon separator)
+ /// field_name: field_type
/// ```
///
/// [struct]: https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#declaring_a_struct_type
/// [tuple]: https://clickhouse.com/docs/en/sql-reference/data-types/tuple
+ /// [databricks]: https://docs.databricks.com/en/sql/language-manual/data-types/struct-type.html
fn parse_struct_field_def(
&mut self,
) -> Result<(StructField, MatchedTrailingBracket), ParserError> {
// Look beyond the next item to infer whether both field name
// and type are specified.
- let is_anonymous_field = !matches!(
+ // Supports both:
+ // - `field_name field_type` (BigQuery style)
+ // - `field_name: field_type` (Databricks/Hive style)
+ let is_named_field = matches!(
(self.peek_nth_token(0).token, self.peek_nth_token(1).token),
- (Token::Word(_), Token::Word(_))
+ (Token::Word(_), Token::Word(_)) | (Token::Word(_), Token::Colon)
);
- let field_name = if is_anonymous_field {
- None
+ let field_name = if is_named_field {
+ let name = self.parse_identifier()?;
+ // Consume optional colon separator (Databricks/Hive style)
Review Comment:
```suggestion
```
##########
src/parser/mod.rs:
##########
@@ -7907,14 +7890,41 @@ impl<'a> Parser<'a> {
pub fn parse_hive_distribution(&mut self) -> Result<HiveDistributionStyle, ParserError> {
if self.parse_keywords(&[Keyword::PARTITIONED, Keyword::BY]) {
self.expect_token(&Token::LParen)?;
- let columns = self.parse_comma_separated(Parser::parse_column_def)?;
+ let columns = self.parse_comma_separated(Parser::parse_column_def_for_partition)?;
self.expect_token(&Token::RParen)?;
Ok(HiveDistributionStyle::PARTITIONED { columns })
} else {
Ok(HiveDistributionStyle::NONE)
}
}
+ /// Parse column definition for PARTITIONED BY clause.
+ ///
+ /// Databricks allows partition columns without types when referencing
+ /// columns already defined in the table specification:
+ /// ```sql
+ /// CREATE TABLE t (col1 STRING, col2 INT) PARTITIONED BY (col1)
+ /// CREATE TABLE t (col1 STRING) PARTITIONED BY (col2 INT)
+ /// ```
+ ///
+ /// See [Databricks](https://docs.databricks.com/en/sql/language-manual/sql-ref-partition.html)
+ fn parse_column_def_for_partition(&mut self) -> Result<ColumnDef, ParserError> {
Review Comment:
Is the reason for this rewrite to support optional datatype? If so I think it would make sense that we introduce a new type that reflects this instead of reusing the `Unspecified` datatype which has a different meaning? e.g. that this function returns
```rust
struct PartitionedByColumnDef { name, data_type: Option<DataType>}`
```
wdyt?
##########
src/parser/mod.rs:
##########
@@ -7907,14 +7890,41 @@ impl<'a> Parser<'a> {
pub fn parse_hive_distribution(&mut self) -> Result<HiveDistributionStyle, ParserError> {
if self.parse_keywords(&[Keyword::PARTITIONED, Keyword::BY]) {
self.expect_token(&Token::LParen)?;
- let columns = self.parse_comma_separated(Parser::parse_column_def)?;
+ let columns = self.parse_comma_separated(Parser::parse_column_def_for_partition)?;
self.expect_token(&Token::RParen)?;
Ok(HiveDistributionStyle::PARTITIONED { columns })
} else {
Ok(HiveDistributionStyle::NONE)
}
}
+ /// Parse column definition for PARTITIONED BY clause.
Review Comment:
```suggestion
/// Parse column definition for `PARTITIONED BY` clause.
```
##########
src/ast/mod.rs:
##########
@@ -4654,22 +4654,34 @@ pub enum Statement {
/// Legacy copy-style options.
options: Vec<CopyLegacyOption>,
},
+ /// ClickHouse:
/// ```sql
/// OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition | PARTITION ID 'partition_id'] [FINAL] [DEDUPLICATE [BY expression]]
/// ```
- ///
/// See ClickHouse <https://clickhouse.com/docs/en/sql-reference/statements/optimize>
+ ///
+ /// Databricks:
+ /// ```sql
+ /// OPTIMIZE table_name [WHERE predicate] [ZORDER BY (col_name1 [, ...])]
+ /// ```
+ /// See Databricks <https://docs.databricks.com/en/sql/language-manual/delta-optimize.html>
OptimizeTable {
/// Table name to optimize.
name: ObjectName,
- /// Optional cluster identifier.
+ /// Whether the `TABLE` keyword was present (ClickHouse uses `OPTIMIZE TABLE`, Databricks uses `OPTIMIZE`).
+ has_table_keyword: bool,
+ /// Optional cluster identifier (ClickHouse).
Review Comment:
```suggestion
/// Optional cluster identifier.
/// [Clickhouse](https://clickhouse.com/docs/en/sql-reference/statements/optimize)
```
##########
src/parser/mod.rs:
##########
@@ -7907,14 +7890,41 @@ impl<'a> Parser<'a> {
pub fn parse_hive_distribution(&mut self) -> Result<HiveDistributionStyle, ParserError> {
if self.parse_keywords(&[Keyword::PARTITIONED, Keyword::BY]) {
self.expect_token(&Token::LParen)?;
- let columns = self.parse_comma_separated(Parser::parse_column_def)?;
+ let columns = self.parse_comma_separated(Parser::parse_column_def_for_partition)?;
self.expect_token(&Token::RParen)?;
Ok(HiveDistributionStyle::PARTITIONED { columns })
} else {
Ok(HiveDistributionStyle::NONE)
}
}
+ /// Parse column definition for PARTITIONED BY clause.
+ ///
+ /// Databricks allows partition columns without types when referencing
+ /// columns already defined in the table specification:
+ /// ```sql
+ /// CREATE TABLE t (col1 STRING, col2 INT) PARTITIONED BY (col1)
+ /// CREATE TABLE t (col1 STRING) PARTITIONED BY (col2 INT)
+ /// ```
+ ///
+ /// See [Databricks](https://docs.databricks.com/en/sql/language-manual/sql-ref-partition.html)
+ fn parse_column_def_for_partition(&mut self) -> Result<ColumnDef, ParserError> {
+ let name = self.parse_identifier()?;
+
+ // Check if the next token indicates there's no type specified
+ // (comma or closing paren means end of this column definition)
+ let data_type = match self.peek_token().token {
+ Token::Comma | Token::RParen => DataType::Unspecified,
+ _ => self.parse_data_type()?,
+ };
Review Comment:
I think we can instead do something like
```rust
let data_type = self.maybe_parse(|parser| parser.parse_data_type())?;
##########
src/ast/mod.rs:
##########
@@ -4654,22 +4654,34 @@ pub enum Statement {
/// Legacy copy-style options.
options: Vec<CopyLegacyOption>,
},
+ /// ClickHouse:
/// ```sql
/// OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition | PARTITION ID 'partition_id'] [FINAL] [DEDUPLICATE [BY expression]]
/// ```
- ///
/// See ClickHouse <https://clickhouse.com/docs/en/sql-reference/statements/optimize>
+ ///
+ /// Databricks:
+ /// ```sql
+ /// OPTIMIZE table_name [WHERE predicate] [ZORDER BY (col_name1 [, ...])]
+ /// ```
+ /// See Databricks <https://docs.databricks.com/en/sql/language-manual/delta-optimize.html>
OptimizeTable {
/// Table name to optimize.
name: ObjectName,
- /// Optional cluster identifier.
+ /// Whether the `TABLE` keyword was present (ClickHouse uses `OPTIMIZE TABLE`, Databricks uses `OPTIMIZE`).
+ has_table_keyword: bool,
+ /// Optional cluster identifier (ClickHouse).
on_cluster: Option<Ident>,
- /// Optional partition spec.
+ /// Optional partition spec (ClickHouse).
partition: Option<Partition>,
- /// Whether `FINAL` was specified.
+ /// Whether `FINAL` was specified (ClickHouse).
include_final: bool,
- /// Optional deduplication settings.
+ /// Optional deduplication settings (ClickHouse).
deduplicate: Option<Deduplicate>,
+ /// Optional WHERE predicate (Databricks).
Review Comment:
```suggestion
/// Optional WHERE predicate.
/// [Databricks](https://docs.databricks.com/en/sql/language-manual/delta-optimize.html)
```
##########
src/parser/mod.rs:
##########
@@ -3361,25 +3334,35 @@ impl<'a> Parser<'a> {
/// Syntax:
///
/// ```sql
+ /// -- BigQuery style
/// [field_name] field_type
+ /// -- Databricks/Hive style (colon separator)
+ /// field_name: field_type
/// ```
///
/// [struct]: https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#declaring_a_struct_type
/// [tuple]: https://clickhouse.com/docs/en/sql-reference/data-types/tuple
+ /// [databricks]: https://docs.databricks.com/en/sql/language-manual/data-types/struct-type.html
fn parse_struct_field_def(
&mut self,
) -> Result<(StructField, MatchedTrailingBracket), ParserError> {
// Look beyond the next item to infer whether both field name
// and type are specified.
- let is_anonymous_field = !matches!(
+ // Supports both:
+ // - `field_name field_type` (BigQuery style)
+ // - `field_name: field_type` (Databricks/Hive style)
Review Comment:
```suggestion
```
thinking we can skip the comment since they're other dialects that are affected, and the code is explanatory in this case
##########
src/ast/mod.rs:
##########
@@ -4654,22 +4654,34 @@ pub enum Statement {
/// Legacy copy-style options.
options: Vec<CopyLegacyOption>,
},
+ /// ClickHouse:
/// ```sql
/// OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition | PARTITION ID 'partition_id'] [FINAL] [DEDUPLICATE [BY expression]]
/// ```
- ///
/// See ClickHouse <https://clickhouse.com/docs/en/sql-reference/statements/optimize>
+ ///
+ /// Databricks:
+ /// ```sql
+ /// OPTIMIZE table_name [WHERE predicate] [ZORDER BY (col_name1 [, ...])]
+ /// ```
+ /// See Databricks <https://docs.databricks.com/en/sql/language-manual/delta-optimize.html>
OptimizeTable {
/// Table name to optimize.
name: ObjectName,
- /// Optional cluster identifier.
+ /// Whether the `TABLE` keyword was present (ClickHouse uses `OPTIMIZE TABLE`, Databricks uses `OPTIMIZE`).
+ has_table_keyword: bool,
+ /// Optional cluster identifier (ClickHouse).
on_cluster: Option<Ident>,
- /// Optional partition spec.
+ /// Optional partition spec (ClickHouse).
partition: Option<Partition>,
- /// Whether `FINAL` was specified.
+ /// Whether `FINAL` was specified (ClickHouse).
Review Comment:
```suggestion
/// Whether `FINAL` was specified.
/// [Clickhouse](https://clickhouse.com/docs/en/sql-reference/statements/optimize)
```
##########
src/ast/mod.rs:
##########
@@ -4654,22 +4654,34 @@ pub enum Statement {
/// Legacy copy-style options.
options: Vec<CopyLegacyOption>,
},
+ /// ClickHouse:
/// ```sql
/// OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition | PARTITION ID 'partition_id'] [FINAL] [DEDUPLICATE [BY expression]]
/// ```
- ///
/// See ClickHouse <https://clickhouse.com/docs/en/sql-reference/statements/optimize>
+ ///
+ /// Databricks:
+ /// ```sql
+ /// OPTIMIZE table_name [WHERE predicate] [ZORDER BY (col_name1 [, ...])]
+ /// ```
+ /// See Databricks <https://docs.databricks.com/en/sql/language-manual/delta-optimize.html>
OptimizeTable {
/// Table name to optimize.
name: ObjectName,
- /// Optional cluster identifier.
+ /// Whether the `TABLE` keyword was present (ClickHouse uses `OPTIMIZE TABLE`, Databricks uses `OPTIMIZE`).
+ has_table_keyword: bool,
+ /// Optional cluster identifier (ClickHouse).
on_cluster: Option<Ident>,
- /// Optional partition spec.
+ /// Optional partition spec (ClickHouse).
partition: Option<Partition>,
- /// Whether `FINAL` was specified.
+ /// Whether `FINAL` was specified (ClickHouse).
include_final: bool,
- /// Optional deduplication settings.
+ /// Optional deduplication settings (ClickHouse).
deduplicate: Option<Deduplicate>,
+ /// Optional WHERE predicate (Databricks).
+ predicate: Option<Expr>,
+ /// Optional ZORDER BY columns (Databricks).
Review Comment:
```suggestion
/// Optional ZORDER BY columns.
/// [Databricks](https://docs.databricks.com/en/sql/language-manual/delta-optimize.html)
```
##########
src/ast/mod.rs:
##########
@@ -4654,22 +4654,34 @@ pub enum Statement {
/// Legacy copy-style options.
options: Vec<CopyLegacyOption>,
},
+ /// ClickHouse:
/// ```sql
/// OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition | PARTITION ID 'partition_id'] [FINAL] [DEDUPLICATE [BY expression]]
/// ```
- ///
/// See ClickHouse <https://clickhouse.com/docs/en/sql-reference/statements/optimize>
+ ///
+ /// Databricks:
+ /// ```sql
+ /// OPTIMIZE table_name [WHERE predicate] [ZORDER BY (col_name1 [, ...])]
+ /// ```
+ /// See Databricks <https://docs.databricks.com/en/sql/language-manual/delta-optimize.html>
OptimizeTable {
/// Table name to optimize.
name: ObjectName,
- /// Optional cluster identifier.
+ /// Whether the `TABLE` keyword was present (ClickHouse uses `OPTIMIZE TABLE`, Databricks uses `OPTIMIZE`).
+ has_table_keyword: bool,
+ /// Optional cluster identifier (ClickHouse).
on_cluster: Option<Ident>,
- /// Optional partition spec.
+ /// Optional partition spec (ClickHouse).
partition: Option<Partition>,
- /// Whether `FINAL` was specified.
+ /// Whether `FINAL` was specified (ClickHouse).
include_final: bool,
- /// Optional deduplication settings.
+ /// Optional deduplication settings (ClickHouse).
Review Comment:
```suggestion
/// Optional deduplication settings.
/// [Clickhouse](https://clickhouse.com/docs/en/sql-reference/statements/optimize)
```
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#issuecomment-3859877293
@funcpp could you take a look when you have some time to resolve the conflicts on the branch?
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "funcpp (via GitHub)" <gi...@apache.org>.
funcpp commented on PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#issuecomment-3868869722
> @funcpp could you take a look when you have some time to resolve the conflicts on the branch?
@iffyio all clean!
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Databricks: Add support for OPTIMIZE, PARTITIONED BY, and STRUCT syntax [datafusion-sqlparser-rs]
Posted by "funcpp (via GitHub)" <gi...@apache.org>.
funcpp commented on PR #2170:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2170#issuecomment-3788144088
> LGTM. I haven't been working in this repo lately, so let's wait for another maintainer to review before merging.
Thanks! @iffyio could you please take a look?
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org