Jefffrey commented on code in PR #5625:
URL: https://github.com/apache/arrow-datafusion/pull/5625#discussion_r1140947905
##########
datafusion/common/src/column.rs:
##########
@@ -112,12 +112,22 @@ impl Column {
/// Serialize column into a quoted flat name string
pub fn quoted_flat_name(&self) -> String {
- // TODO: quote identifiers only when special characters present
- // see: https://github.com/apache/arrow-datafusion/issues/5523
match &self.relation {
Some(r) => {
- format!("{}.{}", r.to_quoted_string(),
quote_identifier(&self.name))
+ let column_name = if self.name.chars().all(|c| c.is_ascii()) {
Review Comment:
the `is_ascii()` check isn't sufficient, as for example if you have a column
with relation `data.base` and name of `column`, then when quoting you'd have to
quote the relation and have output as `"data.base".column`, otherwise the
output would be `data.base.name` which you can see is much more confusing
(which is the relation part and which is the column part?)
it's not limited to only dots. i believe the only time you have unquoted
identifiers is if the identifier is composed of only alphanumeric characters
and underscores, and it doesn't start with a numeric (so `abc_123` can stay
unquoted, but `"123_abc"` must be quoted since begins with a number)
check the official postgres documentation regarding this as we are similar
to postgres in the behaviour:
https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS
--
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]
