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.

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

chore: add object_name_to_table_reference in SqlToRel #5155

Merged
merged 3 commits into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions datafusion/common/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,10 @@ config_namespace! {
config_namespace! {
/// Options related to SQL parser
pub struct SqlParserOptions {
/// Whether to parse float as decimal
/// When set to true, sql parser will parse float as decimal type
pub parse_float_as_decimal: bool, default = false

/// Whether to normalize ident
/// When set to true, sql parser will normalize ident(convert ident to lowercase when not quoted)
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

pub enable_ident_normalization: bool, default = true

}
Expand Down Expand Up @@ -375,6 +375,7 @@ impl ConfigField for ConfigOptions {
self.execution.visit(v, "datafusion.execution", "");
self.optimizer.visit(v, "datafusion.optimizer", "");
self.explain.visit(v, "datafusion.explain", "");
self.sql_parser.visit(v, "datafusion.sql_parser", "");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::engines::datafusion::util::LogicTestContextProvider;
use datafusion::datasource::MemTable;
use datafusion::prelude::SessionContext;
use datafusion_common::{DataFusionError, OwnedTableReference};
use datafusion_sql::planner::{object_name_to_table_reference, SqlToRel};
use datafusion_sql::planner::{object_name_to_table_reference, ParserOptions, SqlToRel};
use sqllogictest::DBOutput;
use sqlparser::ast::{ColumnDef, ObjectName};
use std::sync::Arc;
Expand Down Expand Up @@ -64,7 +64,7 @@ fn create_new_table(
let config = ctx.copied_config();
let sql_to_rel = SqlToRel::new_with_options(
&LogicTestContextProvider {},
datafusion_sql::planner::ParserOptions {
ParserOptions {
parse_float_as_decimal: config
.config_options()
.sql_parser
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ datafusion.optimizer.repartition_joins true
datafusion.optimizer.repartition_windows true
datafusion.optimizer.skip_failed_rules true
datafusion.optimizer.top_down_join_key_reordering true
datafusion.sql_parser.enable_ident_normalization true
datafusion.sql_parser.parse_float_as_decimal false

# show_variable_in_config_options
query R
Expand Down Expand Up @@ -366,4 +368,4 @@ WITH HEADER ROW LOCATION '../../testing/data/csv/aggregate_test_100.csv';
query CCCC
SHOW CREATE TABLE abc;
----
datafusion public abc CREATE EXTERNAL TABLE abc STORED AS CSV LOCATION ../../testing/data/csv/aggregate_test_100.csv
datafusion public abc CREATE EXTERNAL TABLE abc STORED AS CSV LOCATION ../../testing/data/csv/aggregate_test_100.csv
11 changes: 11 additions & 0 deletions datafusion/sql/src/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
other => self.convert_simple_data_type(other),
}
}

fn convert_simple_data_type(&self, sql_type: &SQLDataType) -> Result<DataType> {
match sql_type {
SQLDataType::Boolean => Ok(DataType::Boolean),
Expand Down Expand Up @@ -322,6 +323,16 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
))),
}
}

pub(crate) fn object_name_to_table_reference(
&self,
object_name: ObjectName,
) -> Result<OwnedTableReference> {
object_name_to_table_reference(
object_name,
self.options.enable_ident_normalization,
)
}
}

/// Create a [`OwnedTableReference`] after normalizing the specified ObjectName
Expand Down
9 changes: 2 additions & 7 deletions datafusion/sql/src/relation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
// specific language governing permissions and limitations
// under the License.

use crate::planner::{
object_name_to_table_reference, ContextProvider, PlannerContext, SqlToRel,
};
use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
use datafusion_common::{DataFusionError, Result};
use datafusion_expr::{LogicalPlan, LogicalPlanBuilder};
use sqlparser::ast::TableFactor;
Expand All @@ -33,10 +31,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
let (plan, alias) = match relation {
TableFactor::Table { name, alias, .. } => {
// normalize name and alias
let table_ref = object_name_to_table_reference(
name,
self.options.enable_ident_normalization,
)?;
let table_ref = self.object_name_to_table_reference(name)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

let table_name = table_ref.to_string();
let cte = planner_context.ctes.get(&table_name);
(
Expand Down
48 changes: 10 additions & 38 deletions datafusion/sql/src/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ use crate::parser::{
CreateExternalTable, DFParser, DescribeTableStmt, Statement as DFStatement,
};
use crate::planner::{
object_name_to_qualifier, object_name_to_table_reference, ContextProvider,
PlannerContext, SqlToRel,
object_name_to_qualifier, ContextProvider, PlannerContext, SqlToRel,
};
use crate::utils::normalize_ident;
use arrow_schema::DataType;
Expand Down Expand Up @@ -158,10 +157,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
};

Ok(LogicalPlan::CreateMemoryTable(CreateMemoryTable {
name: object_name_to_table_reference(
name,
self.options.enable_ident_normalization,
)?,
name: self.object_name_to_table_reference(name)?,
input: Arc::new(plan),
if_not_exists,
or_replace,
Expand All @@ -179,10 +175,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
plan = self.apply_expr_alias(plan, columns)?;

Ok(LogicalPlan::CreateView(CreateView {
name: object_name_to_table_reference(
name,
self.options.enable_ident_normalization,
)?,
name: self.object_name_to_table_reference(name)?,
input: Arc::new(plan),
or_replace,
definition: sql,
Expand Down Expand Up @@ -227,10 +220,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
// nor do we support multiple object names
let name = match names.len() {
0 => Err(ParserError("Missing table name.".to_string()).into()),
1 => object_name_to_table_reference(
names.pop().unwrap(),
self.options.enable_ident_normalization,
),
1 => self.object_name_to_table_reference(names.pop().unwrap()),
_ => {
Err(ParserError("Multiple objects not supported".to_string())
.into())
Expand Down Expand Up @@ -421,10 +411,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
statement: DescribeTableStmt,
) -> Result<LogicalPlan> {
let DescribeTableStmt { table_name } = statement;
let table_ref = object_name_to_table_reference(
table_name,
self.options.enable_ident_normalization,
)?;
let table_ref = self.object_name_to_table_reference(table_name)?;

let table_source = self
.schema_provider
Expand Down Expand Up @@ -643,10 +630,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
};

// Do a table lookup to verify the table exists
let table_ref = object_name_to_table_reference(
table_name.clone(),
self.options.enable_ident_normalization,
)?;
let table_ref = self.object_name_to_table_reference(table_name.clone())?;
let provider = self
.schema_provider
.get_table_provider((&table_ref).into())?;
Expand Down Expand Up @@ -698,10 +682,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
};

// Do a table lookup to verify the table exists
let table_name = object_name_to_table_reference(
table_name,
self.options.enable_ident_normalization,
)?;
let table_name = self.object_name_to_table_reference(table_name)?;
let provider = self
.schema_provider
.get_table_provider((&table_name).into())?;
Expand Down Expand Up @@ -803,10 +784,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
source: Box<Query>,
) -> Result<LogicalPlan> {
// Do a table lookup to verify the table exists
let table_name = object_name_to_table_reference(
table_name,
self.options.enable_ident_normalization,
)?;
let table_name = self.object_name_to_table_reference(table_name)?;
let provider = self
.schema_provider
.get_table_provider((&table_name).into())?;
Expand Down Expand Up @@ -914,10 +892,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
);

// Do a table lookup to verify the table exists
let table_ref = object_name_to_table_reference(
sql_table_name,
self.options.enable_ident_normalization,
)?;
let table_ref = self.object_name_to_table_reference(sql_table_name)?;
let _ = self
.schema_provider
.get_table_provider((&table_ref).into())?;
Expand Down Expand Up @@ -955,10 +930,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
);

// Do a table lookup to verify the table exists
let table_ref = object_name_to_table_reference(
sql_table_name,
self.options.enable_ident_normalization,
)?;
let table_ref = self.object_name_to_table_reference(sql_table_name)?;
let _ = self
.schema_provider
.get_table_provider((&table_ref).into())?;
Expand Down
2 changes: 2 additions & 0 deletions docs/source/user-guide/configs.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,5 @@ Environment variables are read during `SessionConfig` initialisation so they mus
| datafusion.optimizer.hash_join_single_partition_threshold | 1048576 | The maximum estimated size in bytes for one input side of a HashJoin will be collected into a single partition |
| datafusion.explain.logical_plan_only | false | When set to true, the explain statement will only print logical plans |
| datafusion.explain.physical_plan_only | false | When set to true, the explain statement will only print physical plans |
| datafusion.sql_parser.parse_float_as_decimal | false | When set to true, sql parser will parse float as decimal type |
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for this too 👍

| datafusion.sql_parser.enable_ident_normalization | true | When set to true, sql parser will normalize ident(convert ident to lowercase when not quoted) |