You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by al...@apache.org on 2022/11/22 12:09:46 UTC
[arrow-datafusion] branch master updated: Fix DESCRIBE statement qualified table issue (#4304)
This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/master by this push:
new 2cc9a07df Fix DESCRIBE statement qualified table issue (#4304)
2cc9a07df is described below
commit 2cc9a07df3efb66759711069f000563b7693ba65
Author: Marko Grujic <ma...@gmail.com>
AuthorDate: Tue Nov 22 13:09:40 2022 +0100
Fix DESCRIBE statement qualified table issue (#4304)
* Fix DESCRIBE statement qualified table issue
* Swap DESCRIBE test parametrization lib with rtest
---
datafusion/core/tests/sql/information_schema.rs | 16 +++++---
datafusion/sql/src/parser.rs | 13 +++----
datafusion/sql/src/planner.rs | 52 ++++++++++++-------------
3 files changed, 43 insertions(+), 38 deletions(-)
diff --git a/datafusion/core/tests/sql/information_schema.rs b/datafusion/core/tests/sql/information_schema.rs
index e06d11fc0..f224e1365 100644
--- a/datafusion/core/tests/sql/information_schema.rs
+++ b/datafusion/core/tests/sql/information_schema.rs
@@ -26,6 +26,8 @@ use datafusion::{
};
use datafusion_expr::Expr;
+use rstest::rstest;
+
use super::*;
#[tokio::test]
@@ -240,16 +242,20 @@ async fn information_schema_show_tables_no_information_schema() {
assert_eq!(err.to_string(), "Error during planning: SHOW TABLES is not supported unless information_schema is enabled");
}
+#[rstest]
+#[case("datafusion.public.some_table")]
+#[case("public.some_table")]
+#[case("some_table")]
#[tokio::test]
-async fn information_schema_describe_table() {
+async fn information_schema_describe_table(#[case] table_name: &str) {
let ctx =
SessionContext::with_config(SessionConfig::new().with_information_schema(true));
- let sql = "CREATE OR REPLACE TABLE y AS VALUES (1,2),(3,4);";
- ctx.sql(sql).await.unwrap();
+ let sql = format!("CREATE OR REPLACE TABLE {table_name} AS VALUES (1,2),(3,4);");
+ ctx.sql(sql.as_str()).await.unwrap();
- let sql_all = "describe y;";
- let results_all = execute_to_batches(&ctx, sql_all).await;
+ let sql_all = format!("DESCRIBE {table_name};");
+ let results_all = execute_to_batches(&ctx, sql_all.as_str()).await;
let expected = vec![
"+-------------+-----------+-------------+",
diff --git a/datafusion/sql/src/parser.rs b/datafusion/sql/src/parser.rs
index 23deafee6..bfbe628f4 100644
--- a/datafusion/sql/src/parser.rs
+++ b/datafusion/sql/src/parser.rs
@@ -20,7 +20,10 @@
//! Declares a SQL parser based on sqlparser that handles custom formats that we need.
use sqlparser::{
- ast::{ColumnDef, ColumnOptionDef, Statement as SQLStatement, TableConstraint},
+ ast::{
+ ColumnDef, ColumnOptionDef, ObjectName, Statement as SQLStatement,
+ TableConstraint,
+ },
dialect::{keywords::Keyword, Dialect, GenericDialect},
parser::{Parser, ParserError},
tokenizer::{Token, Tokenizer},
@@ -84,7 +87,7 @@ impl fmt::Display for CreateExternalTable {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct DescribeTable {
/// Table name
- pub table_name: String,
+ pub table_name: ObjectName,
}
/// DataFusion Statement representations.
@@ -200,11 +203,7 @@ impl<'a> DFParser<'a> {
pub fn parse_describe(&mut self) -> Result<Statement, ParserError> {
let table_name = self.parser.parse_object_name()?;
-
- let des = DescribeTable {
- table_name: table_name.to_string(),
- };
- Ok(Statement::DescribeTable(des))
+ Ok(Statement::DescribeTable(DescribeTable { table_name }))
}
/// Parse a SQL CREATE statement
diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs
index 4053aeb04..a6f43618f 100644
--- a/datafusion/sql/src/planner.rs
+++ b/datafusion/sql/src/planner.rs
@@ -505,16 +505,20 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
&self,
statement: DescribeTable,
) -> Result<LogicalPlan> {
- let table_name = statement.table_name;
+ let table_name = statement.table_name.to_string();
let table_ref: TableReference = table_name.as_str().into();
// check if table_name exists
let _ = self.schema_provider.get_table_provider(table_ref)?;
+ let where_clause = object_name_to_qualifier(&statement.table_name);
+
if self.has_table("information_schema", "tables") {
- let sql = format!("SELECT column_name, data_type, is_nullable \
- FROM information_schema.columns WHERE table_name = '{table_name}';");
- let mut rewrite = DFParser::parse_sql(&sql[..])?;
+ let sql = format!(
+ "SELECT column_name, data_type, is_nullable \
+ FROM information_schema.columns WHERE {where_clause};"
+ );
+ let mut rewrite = DFParser::parse_sql(&sql)?;
self.statement_to_plan(rewrite.pop_front().unwrap())
} else {
Err(DataFusionError::Plan(
@@ -2684,17 +2688,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
let _ = self.schema_provider.get_table_provider(table_ref)?;
// Figure out the where clause
- let columns = vec!["table_name", "table_schema", "table_catalog"].into_iter();
- let where_clause = sql_table_name
- .0
- .iter()
- .rev()
- .zip(columns)
- .map(|(ident, column_name)| {
- format!(r#"{} = '{}'"#, column_name, normalize_ident(ident))
- })
- .collect::<Vec<_>>()
- .join(" AND ");
+ let where_clause = object_name_to_qualifier(sql_table_name);
// treat both FULL and EXTENDED as the same
let select_list = if full || extended {
@@ -2729,17 +2723,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
let _ = self.schema_provider.get_table_provider(table_ref)?;
// Figure out the where clause
- let columns = vec!["table_name", "table_schema", "table_catalog"].into_iter();
- let where_clause = sql_table_name
- .0
- .iter()
- .rev()
- .zip(columns)
- .map(|(ident, column_name)| {
- format!(r#"{} = '{}'"#, column_name, normalize_ident(ident))
- })
- .collect::<Vec<_>>()
- .join(" AND ");
+ let where_clause = object_name_to_qualifier(sql_table_name);
let query = format!(
"SELECT table_catalog, table_schema, table_name, definition FROM information_schema.views WHERE {}",
@@ -2979,6 +2963,22 @@ fn normalize_sql_object_name(sql_object_name: &ObjectName) -> String {
.join(".")
}
+/// Construct a WHERE qualifier suitable for e.g. information_schema filtering
+/// from the provided object identifiers (catalog, schema and table names).
+pub fn object_name_to_qualifier(sql_table_name: &ObjectName) -> String {
+ let columns = vec!["table_name", "table_schema", "table_catalog"].into_iter();
+ sql_table_name
+ .0
+ .iter()
+ .rev()
+ .zip(columns)
+ .map(|(ident, column_name)| {
+ format!(r#"{} = '{}'"#, column_name, normalize_ident(ident))
+ })
+ .collect::<Vec<_>>()
+ .join(" AND ")
+}
+
/// Remove join expressions from a filter expression
fn remove_join_expressions(
expr: &Expr,