Skip to content

Commit

Permalink
fix table reference
Browse files Browse the repository at this point in the history
  • Loading branch information
jiacai2050 committed Feb 14, 2023
1 parent 7c43380 commit c185797
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 10 deletions.
3 changes: 1 addition & 2 deletions server/src/limiter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

use std::{collections::HashSet, sync::RwLock};

use datafusion::catalog::TableReference;
use datafusion_expr::logical_plan::LogicalPlan;
use serde::Serialize;
use serde_derive::Deserialize;
Expand Down Expand Up @@ -106,7 +105,7 @@ impl Limiter {
.try_for_each(|blocked_table| {
if query
.tables
.get(TableReference::from(blocked_table.as_str()))
.get(sql::planner::get_table_ref(blocked_table))
.is_some()
{
BlockedTable {
Expand Down
35 changes: 27 additions & 8 deletions sql/src/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//! Planner converts a SQL AST into logical plans
use std::{
borrow::Cow,
collections::{BTreeMap, HashMap},
convert::TryFrom,
mem,
Expand All @@ -14,6 +15,7 @@ use arrow::{
datatypes::{DataType as ArrowDataType, Field as ArrowField, Schema as ArrowSchema},
error::ArrowError,
};
use catalog::consts::{DEFAULT_CATALOG, DEFAULT_SCHEMA};
use common_types::{
column_schema::{self, ColumnSchema},
datum::{Datum, DatumKind},
Expand All @@ -27,7 +29,10 @@ use datafusion::{
error::DataFusionError,
optimizer::simplify_expressions::{ExprSimplifier, SimplifyContext},
physical_expr::{create_physical_expr, execution_props::ExecutionProps},
sql::planner::{ParserOptions, PlannerContext, SqlToRel},
sql::{
planner::{ParserOptions, PlannerContext, SqlToRel},
ResolvedTableReference,
},
};
use log::{debug, trace};
use snafu::{ensure, Backtrace, OptionExt, ResultExt, Snafu};
Expand Down Expand Up @@ -550,10 +555,9 @@ impl<'a, P: MetaProvider> PlannerDelegate<'a, P> {
// ensure default value options are valid
ensure_column_default_value_valid(table_schema.columns(), &self.meta_provider)?;

// TODO(yingwen): Maybe support create table on other schema?
// TODO: support create table on other catalog/schema
let table_name = stmt.table_name.to_string();
let table_ref = TableReference::from(table_name.as_str());
// Now we only takes the table name and ignore the schema and catalog name
let table_ref = get_table_ref(&table_name);
let table = table_ref.table().to_string();

let plan = CreateTablePlan {
Expand All @@ -572,6 +576,8 @@ impl<'a, P: MetaProvider> PlannerDelegate<'a, P> {
}

fn drop_table_to_plan(&self, stmt: DropTable) -> Result<Plan> {
debug!("Drop table to plan, stmt:{:?}", stmt);

let (table_name, partition_info) =
if let Some(table) = self.find_table(&stmt.table_name.to_string())? {
let table_name = table.name().to_string();
Expand All @@ -587,12 +593,15 @@ impl<'a, P: MetaProvider> PlannerDelegate<'a, P> {
.fail();
};

Ok(Plan::Drop(DropTablePlan {
let plan = DropTablePlan {
engine: stmt.engine,
if_exists: stmt.if_exists,
table: table_name,
partition_info,
}))
};
debug!("Drop table to plan, plan:{:?}", plan);

Ok(Plan::Drop(plan))
}

fn describe_table_to_plan(&self, stmt: DescribeTable) -> Result<Plan> {
Expand Down Expand Up @@ -773,8 +782,7 @@ impl<'a, P: MetaProvider> PlannerDelegate<'a, P> {
}

fn find_table(&self, table_name: &str) -> Result<Option<TableRef>> {
let table_ref = TableReference::from(table_name);

let table_ref = get_table_ref(table_name);
self.meta_provider
.table(table_ref)
.context(MetaProviderFindTable)
Expand Down Expand Up @@ -1071,6 +1079,17 @@ fn ensure_column_default_value_valid<'a, P: MetaProvider>(
Ok(())
}

// Workaroud for TableReference::from(&str)
// it will always convert table to lowercase when not quoted
// TODO: support catalog/schema
pub fn get_table_ref(table_name: &str) -> TableReference {
TableReference::from(ResolvedTableReference {
catalog: Cow::from(DEFAULT_CATALOG),
schema: Cow::from(DEFAULT_SCHEMA),
table: Cow::from(table_name),
})
}

#[cfg(test)]
mod tests {

Expand Down

0 comments on commit c185797

Please sign in to comment.