alamb commented on code in PR #5778:
URL: https://github.com/apache/arrow-datafusion/pull/5778#discussion_r1152197572
##########
datafusion/common/src/dfschema.rs:
##########
@@ -819,11 +841,9 @@ mod tests {
let left = DFSchema::try_from_qualified_schema("t1",
&test_schema_1())?;
let right = DFSchema::try_from_qualified_schema("t1",
&test_schema_1())?;
let join = left.join(&right);
- assert!(join.is_err());
assert_eq!(
- "Schema error: Schema contains duplicate \
- qualified field name \"t1\".\"c0\"",
- &format!("{}", join.err().unwrap())
+ join.unwrap_err().to_string(),
Review Comment:
I cleaned these tests up to use a more standard `unwrap_err()` and
`to_string()` pattern
##########
datafusion/common/src/dfschema.rs:
##########
@@ -872,42 +890,38 @@ mod tests {
let left = DFSchema::try_from_qualified_schema("t1",
&test_schema_1())?;
let right = DFSchema::try_from(test_schema_1())?;
let join = left.join(&right);
- assert!(join.is_err());
- assert_eq!(
- "Schema error: Schema contains qualified \
- field name \"t1\".\"c0\" and unqualified field name \"c0\" which would
be ambiguous",
- &format!("{}", join.err().unwrap())
- );
+ assert_contains!(join.unwrap_err().to_string(),
Review Comment:
I switched to `assert_contains` as that includes the expected and actual in
the message where `assert!(..contains())` just says "false" when it fails
##########
datafusion/common/src/utils.rs:
##########
@@ -166,8 +167,26 @@ where
/// the identifier by replacing it with two double quotes
///
/// e.g. identifier `tab.le"name` becomes `"tab.le""name"`
-pub fn quote_identifier(s: &str) -> String {
- format!("\"{}\"", s.replace('"', "\"\""))
+pub fn quote_identifier(s: &str) -> Cow<str> {
+ if needs_quotes(s) {
Review Comment:
this is the code change
##########
datafusion/common/src/utils.rs:
##########
@@ -464,4 +483,47 @@ mod tests {
Ok(())
}
+
+ #[test]
+ fn test_quote_identifier() -> Result<()> {
Review Comment:
This test verifies that identifiers are properly quoted (and that when
parsed, quoted identifiers are the same)
--
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]
