goldmedal commented on code in PR #21593:
URL: https://github.com/apache/datafusion/pull/21593#discussion_r3079964948
##########
datafusion/sql/src/unparser/dialect.rs:
##########
@@ -211,6 +206,15 @@ pub trait Dialect: Send + Sync {
false
}
+ /// Unparse the unnest plan as `LATERAL FLATTEN(INPUT => expr, ...)`.
+ ///
+ /// Snowflake uses FLATTEN as a table function instead of the SQL-standard
UNNEST.
+ /// When this returns `true`, the unparser emits
+ /// `LATERAL FLATTEN(INPUT => <col>, OUTER => <bool>)` in the FROM clause.
+ fn unnest_as_lateral_flatten(&self) -> bool {
+ false
+ }
Review Comment:
I'd prefer a trait method that returns an Option<TableFactorBuilder> rather
than a boolean flag. Something like (The design of #20648 ):
```rust
fn unparse_unnest_table_factor(
&self,
_unnest: &Unnest,
_columns: &[Ident],
_unparser: &Unparser,
) -> Result<Option<TableFactorBuilder>> {
Ok(None)
}
```
My concern is that `LATERAL FLATTEN(INPUT => <col>, OUTER => <bool>)` is
Snowflake-specific syntax, and this PR embeds a significant amount of that
dialect-specific behavior directly in the core unparser (plan.rs). The unparser
should ideally stay focused on generic SQL generation, with database-specific
behavior delegated to the Dialect trait.
A trait-based approach has a few advantages:
1. Isolation — If Snowflake changes the FLATTEN arguments or syntax, only
SnowflakeDialect needs updating, not the core unparser.
2. Extensibility — Other databases that handle UNNEST differently (e.g.,
Trino's `CROSS JOIN UNNEST`, BigQuery's table-factor `UNNEST`) can implement
the same trait method with their own output, without adding more boolean flags
or conditional branches to plan.rs.
3. Consistency — This follows the existing pattern in the codebase where
dialects override behavior through trait methods that return AST nodes (e.g.,
`scalar_function_to_sql_overrides`), rather than flags that gate hardcoded
logic.
--
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]
