Remove Confusing "Bare" in does not exist messages (#4572)

* Fix Confusing "Bare" in does not exist messages * fmt
apache · Dec 10, 2022 · 4b6ff8d · 4b6ff8d
1 parent 63ec268
commit 4b6ff8d
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 32 deletions.
diff --git a/datafusion/common/src/table_reference.rs b/datafusion/common/src/table_reference.rs
@@ -96,17 +96,20 @@ impl OwnedTableReference {
             },
         }
     }
+}
 
-    /// Return a string suitable for display
-    pub fn display_string(&self) -> String {
+impl std::fmt::Display for OwnedTableReference {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            OwnedTableReference::Bare { table } => table.clone(),
-            OwnedTableReference::Partial { schema, table } => format!("{schema}.{table}"),
+            OwnedTableReference::Bare { table } => write!(f, "{}", table),
+            OwnedTableReference::Partial { schema, table } => {
+                write!(f, "{schema}.{table}")
+            }
             OwnedTableReference::Full {
                 catalog,
                 schema,
                 table,
-            } => format!("{catalog}.{schema}.{table}"),
+            } => write!(f, "{catalog}.{schema}.{table}"),
         }
     }
 }

diff --git a/datafusion/core/src/execution/context.rs b/datafusion/core/src/execution/context.rs
@@ -283,7 +283,7 @@ impl SessionContext {
                         self.register_table(&name, table)?;
                         self.return_empty_dataframe()
                     }
-                    (true, true, Ok(_)) => Err(DataFusionError::Internal(
+                    (true, true, Ok(_)) => Err(DataFusionError::Execution(
                         "'IF NOT EXISTS' cannot coexist with 'REPLACE'".to_string(),
                     )),
                     (_, _, Err(_)) => {
@@ -300,7 +300,7 @@ impl SessionContext {
                         self.return_empty_dataframe()
                     }
                     (false, false, Ok(_)) => Err(DataFusionError::Execution(format!(
-                        "Table '{:?}' already exists",
+                        "Table '{}' already exists",
                         name
                     ))),
                 }
@@ -331,7 +331,7 @@ impl SessionContext {
                         self.return_empty_dataframe()
                     }
                     (false, Ok(_)) => Err(DataFusionError::Execution(format!(
-                        "Table '{:?}' already exists",
+                        "Table '{}' already exists",
                         name
                     ))),
                 }
@@ -345,7 +345,7 @@ impl SessionContext {
                     (Ok(true), _) => self.return_empty_dataframe(),
                     (_, true) => self.return_empty_dataframe(),
                     (_, _) => Err(DataFusionError::Execution(format!(
-                        "Table {:?} doesn't exist.",
+                        "Table '{}' doesn't exist.",
                         name
                     ))),
                 }
@@ -359,7 +359,7 @@ impl SessionContext {
                     (Ok(true), _) => self.return_empty_dataframe(),
                     (_, true) => self.return_empty_dataframe(),
                     (_, _) => Err(DataFusionError::Execution(format!(
-                        "View {:?} doesn't exist.",
+                        "View '{}' doesn't exist.",
                         name
                     ))),
                 }
@@ -451,7 +451,7 @@ impl SessionContext {
                         self.return_empty_dataframe()
                     }
                     (false, Some(_)) => Err(DataFusionError::Execution(format!(
-                        "Schema '{:?}' already exists",
+                        "Schema '{}' already exists",
                         schema_name
                     ))),
                 }
@@ -474,7 +474,7 @@ impl SessionContext {
                         self.return_empty_dataframe()
                     }
                     (false, Some(_)) => Err(DataFusionError::Execution(format!(
-                        "Catalog '{:?}' already exists",
+                        "Catalog '{}' already exists",
                         catalog_name
                     ))),
                 }
@@ -505,7 +505,7 @@ impl SessionContext {
                 self.return_empty_dataframe()
             }
             (false, Ok(_)) => Err(DataFusionError::Execution(format!(
-                "Table '{:?}' already exists",
+                "Table '{}' already exists",
                 cmd.name
             ))),
         }

diff --git a/datafusion/core/tests/sqllogictests/test_files/ddl.slt b/datafusion/core/tests/sqllogictests/test_files/ddl.slt
@@ -59,7 +59,7 @@ statement error Error during planning: table 'datafusion.public.users' not found
 select * from users;
 
 # Can not drop it again
-statement error Table Bare \{ table: "user" \} doesn't exist.
+statement error Table 'user' doesn't exist.
 DROP TABLE user;
 
 # Can not insert into a undefined table
@@ -187,15 +187,15 @@ statement ok
 DROP VIEW bar;
 
 # not ok to drop after already dropped
-statement error View Bare \{ table: "bar" \} doesn't exist.
+statement error View 'bar' doesn't exist.
 DROP VIEW bar;
 
 # ok to drop with IF EXISTS after already dropped
 statement ok
 DROP VIEW IF EXISTS bar;
 
 # can't drop non existent view
-statement error View Bare \{ table: "non_existent_view" \} doesn't exist.
+statement error View 'non_existent_view' doesn't exist.
 DROP VIEW non_existent_view
 
 ##########
@@ -218,15 +218,15 @@ statement ok
 DROP TABLE my_table;
 
 # not ok to drop after already dropped
-statement error Table Bare \{ table: "my_table" \} doesn't exist.
+statement error Table 'my_table' doesn't exist.
 DROP TABLE my_table;
 
 # ok to drop after already dropped with IF EXISTS
 statement ok
 DROP TABLE IF EXISTS my_table;
 
 # Can't drop non existent table
-statement error Table Bare \{ table: "non_existent_table" \} doesn't exist.
+statement error Table 'non_existent_table' doesn't exist.
 DROP TABLE non_existent_table;
 
 
@@ -277,8 +277,7 @@ SELECT * FROM y
 5 6
 
 # 'IF NOT EXISTS' cannot coexist with 'REPLACE'
-# TODO this shoudn't be an internal error
-statement error Internal error: 'IF NOT EXISTS' cannot coexist with 'REPLACE'
+statement error Execution error: 'IF NOT EXISTS' cannot coexist with 'REPLACE'
 CREATE OR REPLACE TABLE if not exists y AS VALUES (7,8);
 
 statement ok
@@ -289,7 +288,7 @@ DROP TABLE y
 statement ok
 CREATE TABLE t AS SELECT 1
 
-statement error View Bare \{ table: "t" \} doesn't exist.
+statement error View 't' doesn't exist.
 DROP VIEW t
 
 statement ok
@@ -299,7 +298,7 @@ DROP TABLE t
 statement ok
 CREATE VIEW v AS SELECT 1;
 
-statement error Table Bare \{ table: "v" \} doesn't exist.
+statement error Table 'v' doesn't exist.
 DROP TABLE v;
 
 statement ok
@@ -382,7 +381,7 @@ DROP TABLE csv_with_timestamps
 statement ok
 CREATE TABLE y AS VALUES (1,2,3);
 
-statement error Table 'Bare \{ table: "y" \}' already exists
+statement error Table 'y' already exists
 CREATE TABLE y AS VALUES (1,2,3);
 
 statement ok
@@ -393,7 +392,7 @@ DROP TABLE y;
 statement ok
 CREATE VIEW y AS VALUES (1,2,3);
 
-statement error Table 'Bare \{ table: "y" \}' already exists
+statement error Table 'y' already exists
 CREATE VIEW y AS VALUES (1,2,3);
 
 statement ok

diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs
@@ -921,7 +921,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
             TableFactor::Table { name, alias, .. } => {
                 // normalize name and alias
                 let table_ref = object_name_to_table_reference(name)?;
-                let table_name = table_ref.display_string();
+                let table_name = table_ref.to_string();
                 let cte = planner_context.ctes.get(&table_name);
                 (
                     match (
@@ -6304,9 +6304,9 @@ mod tests {
 
     #[test]
     fn test_prepare_statement_to_plan_multi_params() {
-        let sql = "PREPARE my_plan(INT, STRING, DOUBLE, INT, DOUBLE, STRING) AS 
-        SELECT id, age, $6  
-        FROM person 
+        let sql = "PREPARE my_plan(INT, STRING, DOUBLE, INT, DOUBLE, STRING) AS
+        SELECT id, age, $6
+        FROM person
         WHERE age IN ($1, $4) AND salary > $3 and salary < $5 OR first_name < $2";
 
         let expected_plan = "Prepare: \"my_plan\" [Int32, Utf8, Float64, Int32, Float64, Utf8] \
@@ -6321,11 +6321,11 @@ mod tests {
 
     #[test]
     fn test_prepare_statement_to_plan_having() {
-        let sql = "PREPARE my_plan(INT, DOUBLE, DOUBLE, DOUBLE) AS 
-        SELECT id, SUM(age)  
+        let sql = "PREPARE my_plan(INT, DOUBLE, DOUBLE, DOUBLE) AS
+        SELECT id, SUM(age)
         FROM person \
-        WHERE salary > $2 
-        GROUP BY id 
+        WHERE salary > $2
+        GROUP BY id
         HAVING sum(age) < $1 AND SUM(age) > 10 OR SUM(age) in ($3, $4)\
         ";