Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Dec 10, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17ea23a
Fix Confusing "Bare" in does not exist messages
alamb Dec 9, 2022
27883d4
fmt
alamb Dec 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions datafusion/common/src/table_reference.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}"),
}
}
}
Expand Down
16 changes: 8 additions & 8 deletions datafusion/core/src/execution/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(_)) => {
Expand All @@ -300,7 +300,7 @@ impl SessionContext {
self.return_empty_dataframe()
}
(false, false, Ok(_)) => Err(DataFusionError::Execution(format!(
"Table '{:?}' already exists",
"Table '{}' already exists",
name
))),
}
Expand Down Expand Up @@ -331,7 +331,7 @@ impl SessionContext {
self.return_empty_dataframe()
}
(false, Ok(_)) => Err(DataFusionError::Execution(format!(
"Table '{:?}' already exists",
"Table '{}' already exists",
name
))),
}
Expand All @@ -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
))),
}
Expand All @@ -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
))),
}
Expand Down Expand Up @@ -451,7 +451,7 @@ impl SessionContext {
self.return_empty_dataframe()
}
(false, Some(_)) => Err(DataFusionError::Execution(format!(
"Schema '{:?}' already exists",
"Schema '{}' already exists",
schema_name
))),
}
Expand All @@ -474,7 +474,7 @@ impl SessionContext {
self.return_empty_dataframe()
}
(false, Some(_)) => Err(DataFusionError::Execution(format!(
"Catalog '{:?}' already exists",
"Catalog '{}' already exists",
catalog_name
))),
}
Expand Down Expand Up @@ -505,7 +505,7 @@ impl SessionContext {
self.return_empty_dataframe()
}
(false, Ok(_)) => Err(DataFusionError::Execution(format!(
"Table '{:?}' already exists",
"Table '{}' already exists",
cmd.name
))),
}
Expand Down
21 changes: 10 additions & 11 deletions datafusion/core/tests/sqllogictests/test_files/ddl.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see the result of the changes in this file ❤️

DROP TABLE user;

# Can not insert into a undefined table
Expand Down Expand Up @@ -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

##########
Expand All @@ -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;


Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 8 additions & 8 deletions datafusion/sql/src/planner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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] \
Expand All @@ -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)\
";

Expand Down