chore: add object_name_to_table_reference in SqlToRel (#5155)

* chore: add object_name_to_table_reference in SqlToRel * update docs * fix broken CI
apache · Feb 3, 2023 · c323721 · c323721
1 parent 953d16b
commit c323721
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 50 deletions.
diff --git a/datafusion/common/src/config.rs b/datafusion/common/src/config.rs
@@ -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)
         pub enable_ident_normalization: bool, default = true
 
     }
@@ -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", "");
     }
 }
 

diff --git a/datafusion/core/tests/sqllogictests/src/engines/datafusion/create_table.rs b/datafusion/core/tests/sqllogictests/src/engines/datafusion/create_table.rs
@@ -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;
@@ -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

diff --git a/datafusion/core/tests/sqllogictests/test_files/information_schema.slt b/datafusion/core/tests/sqllogictests/test_files/information_schema.slt
@@ -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
@@ -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
diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs
@@ -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),
@@ -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

diff --git a/datafusion/sql/src/relation/mod.rs b/datafusion/sql/src/relation/mod.rs
@@ -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;
@@ -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)?;
                 let table_name = table_ref.to_string();
                 let cte = planner_context.ctes.get(&table_name);
                 (

diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs
@@ -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;
@@ -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,
@@ -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,
@@ -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())
@@ -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
@@ -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())?;
@@ -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())?;
@@ -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())?;
@@ -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())?;
@@ -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())?;

diff --git a/docs/source/user-guide/configs.md b/docs/source/user-guide/configs.md
@@ -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                                                                                                                                                                                                                                                 |
+| datafusion.sql_parser.enable_ident_normalization          | true       | When set to true, sql parser will normalize ident(convert ident to lowercase when not quoted)                                                                                                                                                                                                                 |