You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by xu...@apache.org on 2022/12/10 07:07:06 UTC
[arrow-datafusion] branch master updated: Remove Confusing "Bare" in does not exist messages (#4572)
This is an automated email from the ASF dual-hosted git repository.
xudong963 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/master by this push:
new 4b6ff8d92 Remove Confusing "Bare" in does not exist messages (#4572)
4b6ff8d92 is described below
commit 4b6ff8d9283f65cb28c1c5a57f689686f991ec18
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Sat Dec 10 02:07:01 2022 -0500
Remove Confusing "Bare" in does not exist messages (#4572)
* Fix Confusing "Bare" in does not exist messages
* fmt
---
datafusion/common/src/table_reference.rs | 13 ++++++++-----
datafusion/core/src/execution/context.rs | 16 ++++++++--------
.../core/tests/sqllogictests/test_files/ddl.slt | 21 ++++++++++-----------
datafusion/sql/src/planner.rs | 16 ++++++++--------
4 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/datafusion/common/src/table_reference.rs b/datafusion/common/src/table_reference.rs
index 250aa3129..17f1e231d 100644
--- 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
index f54eac6cb..759cc79a8 100644
--- 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
index 20dca0a0a..8af2a9dad 100644
--- 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,7 +187,7 @@ 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
@@ -195,7 +195,7 @@ 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,7 +218,7 @@ 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
@@ -226,7 +226,7 @@ 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
index 6587aa30d..5715ff13e 100644
--- 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)\
";