diff --git a/datafusion/src/sql/planner.rs b/datafusion/src/sql/planner.rs index e668163edec17..bbd5aa7c5696b 100644 --- a/datafusion/src/sql/planner.rs +++ b/datafusion/src/sql/planner.rs @@ -1062,8 +1062,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { } let field = schema.field(field_index - 1); - let col_ident = SQLExpr::Identifier(Ident::new(field.qualified_name())); - self.sql_expr_to_logical_expr(&col_ident, schema)? + Expr::Column(field.qualified_column()) } e => self.sql_expr_to_logical_expr(e, schema)?, }; @@ -1323,9 +1322,14 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { let var_names = vec![id.value.clone()]; Ok(Expr::ScalarVariable(var_names)) } else { - // create a column expression based on raw user input, this column will be - // normalized with qualifer later by the SQL planner. - Ok(col(&id.value)) + // Don't use `col()` here because it will try to + // interpret names with '.' as if they were + // compound indenfiers, but this is not a compound + // identifier. (e.g. it is "foo.bar" not foo.bar) + Ok(Expr::Column(Column { + relation: None, + name: id.value.clone(), + })) } } diff --git a/datafusion/tests/sql.rs b/datafusion/tests/sql.rs index 945bb7ebc2eb8..481186b5e12e5 100644 --- a/datafusion/tests/sql.rs +++ b/datafusion/tests/sql.rs @@ -5426,6 +5426,86 @@ async fn qualified_table_references() -> Result<()> { Ok(()) } +#[tokio::test] +async fn qualified_table_references_and_fields() -> Result<()> { + let mut ctx = ExecutionContext::new(); + + let c1: StringArray = vec!["foofoo", "foobar", "foobaz"] + .into_iter() + .map(Some) + .collect(); + let c2: Int64Array = vec![1, 2, 3].into_iter().map(Some).collect(); + let c3: Int64Array = vec![10, 20, 30].into_iter().map(Some).collect(); + + let batch = RecordBatch::try_from_iter(vec![ + ("f.c1", Arc::new(c1) as ArrayRef), + // evil -- use the same name as the table + ("test.c2", Arc::new(c2) as ArrayRef), + // more evil still + ("....", Arc::new(c3) as ArrayRef), + ])?; + + let table = MemTable::try_new(batch.schema(), vec![vec![batch]])?; + ctx.register_table("test", Arc::new(table))?; + + // referring to the unquoted column is an error + let sql = r#"SELECT f1.c1 from test"#; + let error = ctx.create_logical_plan(sql).unwrap_err(); + assert_contains!( + error.to_string(), + "No field named 'f1.c1'. Valid fields are 'test.f.c1', 'test.test.c2'" + ); + + // however, enclosing it in double quotes is ok + let sql = r#"SELECT "f.c1" from test"#; + let actual = execute_to_batches(&mut ctx, sql).await; + let expected = vec![ + "+--------+", + "| f.c1 |", + "+--------+", + "| foofoo |", + "| foobar |", + "| foobaz |", + "+--------+", + ]; + assert_batches_eq!(expected, &actual); + // Works fully qualified too + let sql = r#"SELECT test."f.c1" from test"#; + let actual = execute_to_batches(&mut ctx, sql).await; + assert_batches_eq!(expected, &actual); + + // check that duplicated table name and column name are ok + let sql = r#"SELECT "test.c2" as expr1, test."test.c2" as expr2 from test"#; + let actual = execute_to_batches(&mut ctx, sql).await; + let expected = vec![ + "+-------+-------+", + "| expr1 | expr2 |", + "+-------+-------+", + "| 1 | 1 |", + "| 2 | 2 |", + "| 3 | 3 |", + "+-------+-------+", + ]; + assert_batches_eq!(expected, &actual); + + // check that '....' is also an ok column name (in the sense that + // datafusion should run the query, not that someone should write + // this + let sql = r#"SELECT "....", "...." as c3 from test order by "....""#; + let actual = execute_to_batches(&mut ctx, sql).await; + let expected = vec![ + "+------+----+", + "| .... | c3 |", + "+------+----+", + "| 10 | 10 |", + "| 20 | 20 |", + "| 30 | 30 |", + "+------+----+", + ]; + assert_batches_eq!(expected, &actual); + Ok(()) +} + #[tokio::test] async fn invalid_qualified_table_references() -> Result<()> { let mut ctx = ExecutionContext::new();