xitep commented on code in PR #2101:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2101#discussion_r2559364090
##########
src/dialect/mod.rs:
##########
@@ -601,13 +601,122 @@ pub trait Dialect: Debug + Any {
false
}
- /// Return true if the dialect supports specifying multiple options
+ /// Returns true if the dialect supports specifying multiple options
/// in a `CREATE TABLE` statement for the structure of the new table. For
example:
/// `CREATE TABLE t (a INT, b INT) AS SELECT 1 AS b, 2 AS a`
fn supports_create_table_multi_schema_info_sources(&self) -> bool {
false
}
+ /// Returns `true` if the dialect supports qualified column names
Review Comment:
* postgresql (https://www.postgresql.org/docs/current/sql-merge.html) ...
"[..] The name of a column in the target table. The column name can be
qualified with a subfield name or array subscript, if needed. (Inserting into
only some fields of a composite column leaves the other fields null.) Do not
include the table's name in the specification of a target column. [..]"
* mssql:
https://learn.microsoft.com/en-us/sql/t-sql/statements/merge-transact-sql?view=sql-server-ver17
... "[..] Columns must be specified as a single-part name. [..]"
* snowflakes' document
(https://docs.snowflake.com/en/sql-reference/sql/merge) is not really clear on
the details apart of "... specifies one or more columns in the target table to
be inserted ..." :/
* databricks:
https://docs.databricks.com/aws/en/sql/language-manual/delta-merge-into ... is
also not very clear, but i get that it's column names of the target table.
* duckdb: https://duckdb.org/docs/stable/sql/statements/merge_into ... looks
like a plain old column name, ie. a plain identifier.
* bigquery:
https://docs.cloud.google.com/bigquery/docs/reference/standard-sql/dml-syntax#merge_statement
... looks like plain "column name"
* redshift: https://docs.aws.amazon.com/redshift/latest/dg/r_MERGE.html ...
One or more column names that you want to modify. Don't include the table name
when specifying the target column.
* hive: https://hive.apache.org/docs/latest/language/languagemanual-dml/
seems to not to support a column in the first place
so, apart of postgresql, a qualified identifier (ie. `ObjectName`) should be
fine. this would allow me to off-load the entire validation from
`sqlparser-rs` into the client program, which i believe is desirable.
##########
src/dialect/mod.rs:
##########
@@ -601,13 +601,122 @@ pub trait Dialect: Debug + Any {
false
}
- /// Return true if the dialect supports specifying multiple options
+ /// Returns true if the dialect supports specifying multiple options
/// in a `CREATE TABLE` statement for the structure of the new table. For
example:
/// `CREATE TABLE t (a INT, b INT) AS SELECT 1 AS b, 2 AS a`
fn supports_create_table_multi_schema_info_sources(&self) -> bool {
false
}
+ /// Returns `true` if the dialect supports qualified column names
+ /// as part of a MERGE's INSERT's column list. Example:
+ ///
+ /// ```sql
+ /// MERGE INTO FOO
+ /// USING FOO_IMP
+ /// ON (FOO.ID = FOO_IMP.ID)
+ /// WHEN NOT MATCHED THEN
+ /// -- no qualifier
+ /// INSERT (ID, NAME)
+ /// VALUES (FOO_IMP.ID, UPPER(FOO_IMP.NAME))
+ /// ```
+ /// vs.
+ /// ```sql
+ /// MERGE INTO FOO
+ /// USING FOO_IMP
+ /// ON (FOO.ID = FOO_IMP.ID)
+ /// WHEN NOT MATCHED THEN
+ /// -- here: qualified
+ /// INSERT (FOO.ID, FOO.NAME)
+ /// VALUES (FOO_IMP.ID, UPPER(FOO_IMP.NAME))
+ /// ```
+ /// or
+ /// ```sql
+ /// MERGE INTO FOO X
+ /// USING FOO_IMP
+ /// ON (X.ID = FOO_IMP.ID)
+ /// WHEN NOT MATCHED THEN
+ /// -- here: qualified using the alias
+ /// INSERT (X.ID, X.NAME)
+ /// VALUES (FOO_IMP.ID, UPPER(FOO_IMP.NAME))
+ /// ```
+ ///
+ /// Note: in the latter case, the qualifier must match the target table
+ /// name or its alias if one is present. The parser will enforce this.
+ ///
+ /// The default implementation always returns `false` not allowing the
+ /// qualifiers.
+ fn supports_merge_insert_qualified_columns(&self) -> bool {
+ false
+ }
+
+ /// Returns `true` if the dialect supports specify an INSERT predicate in
Review Comment:
i fear i don't follow entirely :-/
do you mean to change the default impl to always return `true`? (i did that
for the `GenericDialect`, but kept the status-quo for all the others.)
##########
src/dialect/mod.rs:
##########
@@ -601,13 +601,122 @@ pub trait Dialect: Debug + Any {
false
}
- /// Return true if the dialect supports specifying multiple options
+ /// Returns true if the dialect supports specifying multiple options
/// in a `CREATE TABLE` statement for the structure of the new table. For
example:
/// `CREATE TABLE t (a INT, b INT) AS SELECT 1 AS b, 2 AS a`
fn supports_create_table_multi_schema_info_sources(&self) -> bool {
false
}
+ /// Returns `true` if the dialect supports qualified column names
+ /// as part of a MERGE's INSERT's column list. Example:
+ ///
+ /// ```sql
+ /// MERGE INTO FOO
+ /// USING FOO_IMP
+ /// ON (FOO.ID = FOO_IMP.ID)
+ /// WHEN NOT MATCHED THEN
+ /// -- no qualifier
+ /// INSERT (ID, NAME)
+ /// VALUES (FOO_IMP.ID, UPPER(FOO_IMP.NAME))
+ /// ```
+ /// vs.
+ /// ```sql
+ /// MERGE INTO FOO
+ /// USING FOO_IMP
+ /// ON (FOO.ID = FOO_IMP.ID)
+ /// WHEN NOT MATCHED THEN
+ /// -- here: qualified
+ /// INSERT (FOO.ID, FOO.NAME)
+ /// VALUES (FOO_IMP.ID, UPPER(FOO_IMP.NAME))
+ /// ```
+ /// or
+ /// ```sql
+ /// MERGE INTO FOO X
+ /// USING FOO_IMP
+ /// ON (X.ID = FOO_IMP.ID)
+ /// WHEN NOT MATCHED THEN
+ /// -- here: qualified using the alias
+ /// INSERT (X.ID, X.NAME)
+ /// VALUES (FOO_IMP.ID, UPPER(FOO_IMP.NAME))
+ /// ```
+ ///
+ /// Note: in the latter case, the qualifier must match the target table
+ /// name or its alias if one is present. The parser will enforce this.
Review Comment:
yes, indeed that's what i'd prefer. as mentioned elsewhere this (whole
validation) was a compromise so that would be able to preserved the columns
type as `Ident`. I understand that you would be open to change it. :)
as you mentioned above in the other comment, this validation _is_ semantics
and I'd be happy to have it outside of the (general purpose) parser.
--
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]
