jackwener commented on code in PR #4484:
URL: https://github.com/apache/arrow-datafusion/pull/4484#discussion_r1037971421
##########
datafusion/sql/src/planner.rs:
##########
@@ -852,24 +836,12 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
// normalize name and alias
let table_name = normalize_sql_object_name(sql_object_name);
let table_ref: TableReference = table_name.as_str().into();
- let table_alias = alias.as_ref().map(|a|
normalize_ident(&a.name));
let cte = ctes.get(&table_name);
(
match (cte,
self.schema_provider.get_table_provider(table_ref)) {
- (Some(cte_plan), _) => match table_alias {
- Some(cte_alias) => {
- Ok(with_alias(cte_plan.clone(), cte_alias))
- }
- _ => Ok(cte_plan.clone()),
- },
+ (Some(cte_plan), _) => Ok(cte_plan.clone()),
(_, Ok(provider)) => {
- let scan =
- LogicalPlanBuilder::scan(&table_name,
provider, None);
- let scan = match table_alias.as_ref() {
- Some(ref name) =>
scan?.alias(name.to_owned().as_str()),
- _ => scan,
- };
- scan?.build()
Review Comment:
We must remove these code, because they will add `SubqueryAlias`.
But following code will be added once again, it will cause replicated
`SubqueryAlias`
##########
datafusion/sql/src/planner.rs:
##########
@@ -921,25 +887,31 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
plan: LogicalPlan,
alias: TableAlias,
) -> Result<LogicalPlan> {
- let columns_alias = alias.clone().columns;
- if columns_alias.is_empty() {
- // sqlparser-rs encodes AS t as an empty list of column alias
Review Comment:
Original code is bug (add twice) + bug (when columns-alias, forget to add
alias). let me have explain.
The process is as follows
- Add table-alias (above those code are removed)
- Add table-alias and columns-alias (these code here)
- bug: when columns-alias is empty, these code will ignore table-alias
so we just unify them into
- Add table-alias and columns-alias
##########
datafusion/sql/src/planner.rs:
##########
@@ -879,19 +851,13 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
TableFactor::Derived {
subquery, alias, ..
} => {
- let normalized_alias = alias.as_ref().map(|a|
normalize_ident(&a.name));
let logical_plan = self.query_to_plan_with_alias(
*subquery,
None,
ctes,
outer_query_schema,
)?;
-
- let plan = match normalized_alias {
- Some(alias) => with_alias(logical_plan, alias),
- _ => logical_plan,
- };
- (plan, alias)
Review Comment:
We must remove these code, because they will add SubqueryAlias.
But following code will be added once again, it will cause replicated
SubqueryAlias
##########
benchmarks/expected-plans/q13.txt:
##########
@@ -1,12 +1,11 @@
-Sort: custdist DESC NULLS FIRST, c_orders.c_count DESC NULLS FIRST
- Projection: c_orders.c_count, COUNT(UInt8(1)) AS custdist
- Aggregate: groupBy=[[c_orders.c_count]], aggr=[[COUNT(UInt8(1))]]
- SubqueryAlias: c_orders
Review Comment:
below existed `SubqueryAlias: c_orders`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
