Skip to content

fix: unparse for subqueryalias #15068

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Mar 11, 2025
Merged

fix: unparse for subqueryalias #15068

merged 3 commits into from
Mar 11, 2025

Conversation

chenkovsky
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

the qualifier is incorrect for subqueryalias when unparsing.

What changes are included in this PR?

wrap with subqueryalias if alias is passed when unparsing.

Are these changes tested?

Unittest

Are there any user-facing changes?

No

@github-actions github-actions bot added sql SQL Planner core Core DataFusion crate labels Mar 7, 2025
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @chenkovsky

Comment on lines 987 to 997
let ret = Self::unparse_table_scan_pushdown(
&subquery_alias.input,
Some(subquery_alias.alias.clone()),
already_projected,
)
);
if let Some(alias) = alias {
if let Ok(Some(plan)) = ret {
let plan = LogicalPlanBuilder::new(plan).alias(alias)?.build()?;
return Ok(Some(plan));
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can write this a bit more concisely by unwrapping the error earlier if you wanted:

Something like

Suggested change
let ret = Self::unparse_table_scan_pushdown(
&subquery_alias.input,
Some(subquery_alias.alias.clone()),
already_projected,
)
);
if let Some(alias) = alias {
if let Ok(Some(plan)) = ret {
let plan = LogicalPlanBuilder::new(plan).alias(alias)?.build()?;
return Ok(Some(plan));
}
}
)?;
if let Some(alias) = alias {
let plan = LogicalPlanBuilder::new(plan).alias(alias)?.build()?;
return Ok(Some(plan));
}
Ok(ret)

ctx.register_batch("customer", customer())?;
let plan = ctx.sql(sql).await?.into_optimized_plan()?;
let sql = plan_to_sql(&plan)?;
assert_eq!(sql.to_string(), "SELECT customer_view.c_custkey, customer_view.c_name, customer_view.custkey_plus FROM (SELECT customer.c_custkey, (CAST(customer.c_custkey AS BIGINT) + 1) AS custkey_plus, customer.c_name FROM (SELECT customer.c_custkey, customer.c_name FROM customer AS customer) AS customer) AS customer_view");
Copy link
Contributor

Choose a reason for hiding this comment

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

I parsed this with some structure and it looks right to me

Thanks @chenkovsky

 "SELECT 
   customer_view.c_custkey, 
   customer_view.c_name, 
   customer_view.custkey_plus 
   FROM 
   (SELECT 
     customer.c_custkey, 
     (CAST(customer.c_custkey AS BIGINT) + 1) AS custkey_plus, 
     customer.c_name 
     FROM 
     (SELECT 
       customer.c_custkey, 
       customer.c_name 
       FROM customer AS customer
       ) AS customer
       ) AS customer_view");

@github-actions github-actions bot removed the core Core DataFusion crate label Mar 10, 2025
@goldmedal goldmedal merged commit ee77d58 into apache:main Mar 11, 2025
24 checks passed
@goldmedal
Copy link
Contributor

Thanks @chenkovsky and @alamb for reviewing 👍

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
sql SQL Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unparse SubqueryAlias with the pushdown TableScan fail
3 participants