Jefffrey commented on code in PR #5183:
URL: https://github.com/apache/arrow-datafusion/pull/5183#discussion_r1096603578
##########
datafusion/common/src/table_reference.rs:
##########
@@ -155,50 +178,110 @@ impl<'a> TableReference<'a> {
table,
},
Self::Partial { schema, table } => ResolvedTableReference {
- catalog: default_catalog,
+ catalog: default_catalog.into(),
schema,
table,
},
Self::Bare { table } => ResolvedTableReference {
- catalog: default_catalog,
- schema: default_schema,
+ catalog: default_catalog.into(),
+ schema: default_schema.into(),
table,
},
}
}
- /// Forms a [`TableReference`] by splitting `s` on periods `.`.
- ///
- /// Note that this function does NOT handle periods or name
- /// normalization correctly (e.g. `"foo.bar"` will be parsed as
- /// `"foo`.`bar"`. and `Foo` will be parsed as `Foo` (not `foo`).
- ///
- /// If you need to handle such identifiers correctly, you should
- /// use a SQL parser or form the [`OwnedTableReference`] directly.
+ /// Forms a [`TableReference`] by attempting to parse `s` as a multipart
identifier,
+ /// failing that then taking the entire input as the identifier itself.
///
- /// See more detail in
<https://github.com/apache/arrow-datafusion/issues/4532>
+ /// Will normalize (convert to lowercase) any unquoted identifiers.
+ /// e.g. `Foo` will be parsed as `foo`, and `"Foo"".bar"` will be parsed as
+ /// `Foo".bar` (note the preserved case and requiring two double quotes to
represent
+ /// a single double quote in the identifier)
pub fn parse_str(s: &'a str) -> Self {
- let parts: Vec<&str> = s.split('.').collect();
+ let mut parts = parse_identifiers(s)
+ .unwrap_or(vec![])
+ .into_iter()
+ .map(|id| match id.quote_style {
+ Some(_) => id.value,
+ // TODO: someway to be able to toggle this functionality?
+ None => id.value.to_ascii_lowercase(),
+ })
Review Comment:
comparing with datafusion-sql package where some functionality is duplicated:
https://github.com/apache/arrow-datafusion/blob/ac876dbc9729b16e272e00496c51e53d9f649173/datafusion/sql/src/utils.rs#L540-L546
unsure if also should have a way to toggle normalization, as for
datafusion-sql package it allows it:
https://github.com/apache/arrow-datafusion/blob/c323721192ba2733a56c4201b2255a36b1eaa859/datafusion/sql/src/planner.rs#L424-L430
but since this `parse_str` is meant to be used to implement `From<&str>`,
there wouldn't really be a way to pass information on whether to toggle or not
--
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]
