Skip to content

Commit

Permalink
Fix CREATE SCHEMA schema name double quoting issue. (#5059)
Browse files Browse the repository at this point in the history
  • Loading branch information
neumark authored Jan 26, 2023
1 parent e24a7ec commit 8b716d3
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 10 deletions.
47 changes: 37 additions & 10 deletions datafusion/sql/src/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use crate::planner::{
object_name_to_qualifier, object_name_to_table_reference, ContextProvider,
PlannerContext, SqlToRel,
};
use crate::utils::normalize_ident;
use arrow_schema::DataType;
use datafusion_common::parsers::CompressionTypeVariant;
use datafusion_common::{
Expand All @@ -40,14 +41,39 @@ use datafusion_expr::{
};
use sqlparser::ast;
use sqlparser::ast::{
Assignment, Expr as SQLExpr, Expr, Ident, ObjectName, ObjectType, Query, SetExpr,
ShowCreateObject, ShowStatementFilter, Statement, TableFactor, TableWithJoins,
UnaryOperator, Value,
Assignment, Expr as SQLExpr, Expr, Ident, ObjectName, ObjectType, Query, SchemaName,
SetExpr, ShowCreateObject, ShowStatementFilter, Statement, TableFactor,
TableWithJoins, UnaryOperator, Value,
};
use sqlparser::parser::ParserError::ParserError;
use std::collections::{BTreeMap, HashMap, HashSet};
use std::sync::Arc;

fn ident_to_string(ident: &Ident) -> String {
normalize_ident(ident.to_owned())
}

fn object_name_to_string(object_name: &ObjectName) -> String {
object_name
.0
.iter()
.map(ident_to_string)
.collect::<Vec<String>>()
.join(".")
}

fn get_schema_name(schema_name: &SchemaName) -> String {
match schema_name {
SchemaName::Simple(schema_name) => object_name_to_string(schema_name),
SchemaName::UnnamedAuthorization(auth) => ident_to_string(auth),
SchemaName::NamedAuthorization(schema_name, auth) => format!(
"{}.{}",
object_name_to_string(schema_name),
ident_to_string(auth)
),
}
}

impl<'a, S: ContextProvider> SqlToRel<'a, S> {
/// Generate a logical plan from an DataFusion SQL statement
pub fn statement_to_plan(&self, statement: DFStatement) -> Result<LogicalPlan> {
Expand Down Expand Up @@ -170,7 +196,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
schema_name,
if_not_exists,
} => Ok(LogicalPlan::CreateCatalogSchema(CreateCatalogSchema {
schema_name: schema_name.to_string(),
schema_name: get_schema_name(&schema_name),
if_not_exists,
schema: Arc::new(DFSchema::empty()),
})),
Expand All @@ -179,7 +205,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
if_not_exists,
..
} => Ok(LogicalPlan::CreateCatalog(CreateCatalog {
catalog_name: db_name.to_string(),
catalog_name: object_name_to_string(&db_name),
if_not_exists,
schema: Arc::new(DFSchema::empty()),
})),
Expand Down Expand Up @@ -240,7 +266,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
&mut planner_context,
)?;
Ok(LogicalPlan::Prepare(Prepare {
name: name.to_string(),
name: ident_to_string(&name),
data_types,
input: Arc::new(plan),
}))
Expand Down Expand Up @@ -487,7 +513,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
}

fn show_variable_to_plan(&self, variable: &[Ident]) -> Result<LogicalPlan> {
let variable = ObjectName(variable.to_vec()).to_string();
let variable = object_name_to_string(&ObjectName(variable.to_vec()));

if !self.has_table("information_schema", "df_settings") {
return Err(DataFusionError::Plan(
Expand Down Expand Up @@ -537,7 +563,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
));
}

let variable = variable.to_string();
let variable = object_name_to_string(variable);
let mut variable_lower = variable.to_lowercase();

if variable_lower == "timezone" || variable_lower == "time.zone" {
Expand All @@ -547,7 +573,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {

// parse value string from Expr
let value_string = match &value[0] {
SQLExpr::Identifier(i) => i.to_string(),
SQLExpr::Identifier(i) => ident_to_string(i),
SQLExpr::Value(v) => match v {
Value::SingleQuotedString(s) => s.to_string(),
Value::DollarQuotedString(s) => s.to_string(),
Expand Down Expand Up @@ -611,7 +637,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
let schema = (*provider.schema()).clone();
let schema = DFSchema::try_from(schema)?;
let scan =
LogicalPlanBuilder::scan(table_name.to_string(), provider, None)?.build()?;
LogicalPlanBuilder::scan(object_name_to_string(&table_name), provider, None)?
.build()?;
let mut planner_context = PlannerContext::new();

let source = match predicate_expr {
Expand Down
21 changes: 21 additions & 0 deletions datafusion/sql/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,27 @@ fn create_external_table_csv() {
quick_test(sql, expected);
}

#[test]
fn create_schema_with_quoted_name() {
let sql = "CREATE SCHEMA \"quoted_schema_name\"";
let expected = "CreateCatalogSchema: \"quoted_schema_name\"";
quick_test(sql, expected);
}

#[test]
fn create_schema_with_quoted_unnormalized_name() {
let sql = "CREATE SCHEMA \"Foo\"";
let expected = "CreateCatalogSchema: \"Foo\"";
quick_test(sql, expected);
}

#[test]
fn create_schema_with_unquoted_normalized_name() {
let sql = "CREATE SCHEMA Foo";
let expected = "CreateCatalogSchema: \"foo\"";
quick_test(sql, expected);
}

#[test]
fn create_external_table_custom() {
let sql = "CREATE EXTERNAL TABLE dt STORED AS DELTATABLE LOCATION 's3://bucket/schema/table';";
Expand Down

0 comments on commit 8b716d3

Please sign in to comment.