You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "sgrebnov (via GitHub)" <gi...@apache.org> on 2024/09/16 05:55:48 UTC
[PR] Improve SQLite subquery tables aliasing unparsing [datafusion]
sgrebnov opened a new pull request, #12482:
URL: https://github.com/apache/datafusion/pull/12482
## Which issue does this PR close?
Follow up PR for https://github.com/apache/datafusion/pull/12331 that improves SQLite subquery tables aliasing unparsing to support more complex cases, for example
```sql
SELECT * FROM (SELECT o_orderkey + 1 FROM orders) AS c(key) LIMIT 10
SELECT * FROM (SELECT o_orderkey FROM orders LIMIT 10) AS c(key) LIMIT 10
```
## What changes are included in this PR?
PRs modifies `LogicalPlan::SubqueryAlias(plan_alias)` unparser to support more complex table aliasing scenarios for SQLite:
1 - calculated columns
2- cases where subquery projection is wrapped by other operators (e.g., LIMIT, SORT)
## Are these changes tested?
Tested manually and added unit tests
## Are there any user-facing changes?
No
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve SQLite subquery tables aliasing unparsing [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #12482:
URL: https://github.com/apache/datafusion/pull/12482
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve SQLite subquery tables aliasing unparsing [datafusion]
Posted by "sgrebnov (via GitHub)" <gi...@apache.org>.
sgrebnov commented on code in PR #12482:
URL: https://github.com/apache/datafusion/pull/12482#discussion_r1761390609
##########
datafusion/sql/src/unparser/rewrite.rs:
##########
@@ -258,8 +258,36 @@ pub(super) fn subquery_alias_inner_query_and_columns(
(outer_projections.input.as_ref(), columns)
}
-/// Injects column aliases into the projection of a logical plan by wrapping `Expr::Column` expressions
-/// with `Expr::Alias` using the provided list of aliases. Non-column expressions are left unchanged.
+/// Injects column aliases into a subquery's logical plan. The function searches for a `Projection`
+/// within the given plan, which may be wrapped by other operators (e.g., LIMIT, SORT).
+/// If the top-level plan is a `Projection`, it directly injects the column aliases.
+/// Otherwise, it iterates through the plan's children to locate and transform the `Projection`.
+///
+/// Example:
+/// - `SELECT col1, col2 FROM table LIMIT 10` plan with aliases `["alias_1", "some_alias_2"]` will be transformed to
+/// - `SELECT col1 AS alias_1, col2 AS some_alias_2 FROM table LIMIT 10`
+pub(super) fn inject_column_aliases_into_subquery(
+ plan: LogicalPlan,
+ aliases: Vec<Ident>,
+) -> Result<LogicalPlan> {
+ match &plan {
+ LogicalPlan::Projection(inner_p) => Ok(inject_column_aliases(inner_p, aliases)),
+ _ => {
+ // projection is wrapped by other operator (LIMIT, SORT, etc), iterate through the plan to find it
+ plan.map_children(|child| {
+ if let LogicalPlan::Projection(p) = &child {
+ Ok(Transformed::yes(inject_column_aliases(p, aliases.clone())))
Review Comment:
@dmitrybugakov - good point, but the aliases will actually be cloned only once for the subquery projection, which can only be singular. I also reviewed whether it makes sense to borrow aliases and clone them in `inject_column_aliases`, but allowing the function to consume the provided aliases seems the right approach so it can be used without extra aliases cloning inside.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve SQLite subquery tables aliasing unparsing [datafusion]
Posted by "sgrebnov (via GitHub)" <gi...@apache.org>.
sgrebnov commented on code in PR #12482:
URL: https://github.com/apache/datafusion/pull/12482#discussion_r1761390609
##########
datafusion/sql/src/unparser/rewrite.rs:
##########
@@ -258,8 +258,36 @@ pub(super) fn subquery_alias_inner_query_and_columns(
(outer_projections.input.as_ref(), columns)
}
-/// Injects column aliases into the projection of a logical plan by wrapping `Expr::Column` expressions
-/// with `Expr::Alias` using the provided list of aliases. Non-column expressions are left unchanged.
+/// Injects column aliases into a subquery's logical plan. The function searches for a `Projection`
+/// within the given plan, which may be wrapped by other operators (e.g., LIMIT, SORT).
+/// If the top-level plan is a `Projection`, it directly injects the column aliases.
+/// Otherwise, it iterates through the plan's children to locate and transform the `Projection`.
+///
+/// Example:
+/// - `SELECT col1, col2 FROM table LIMIT 10` plan with aliases `["alias_1", "some_alias_2"]` will be transformed to
+/// - `SELECT col1 AS alias_1, col2 AS some_alias_2 FROM table LIMIT 10`
+pub(super) fn inject_column_aliases_into_subquery(
+ plan: LogicalPlan,
+ aliases: Vec<Ident>,
+) -> Result<LogicalPlan> {
+ match &plan {
+ LogicalPlan::Projection(inner_p) => Ok(inject_column_aliases(inner_p, aliases)),
+ _ => {
+ // projection is wrapped by other operator (LIMIT, SORT, etc), iterate through the plan to find it
+ plan.map_children(|child| {
+ if let LogicalPlan::Projection(p) = &child {
+ Ok(Transformed::yes(inject_column_aliases(p, aliases.clone())))
Review Comment:
@dmitrybugakov - good point, the aliases will actually be cloned only once for the subquery projection, which can only be singular. I also reviewed whether it makes sense to borrow aliases and clone them in `inject_column_aliases`, but allowing the function to consume the provided aliases seems the right approach so it can be used without extra aliases cloning inside.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve SQLite subquery tables aliasing unparsing [datafusion]
Posted by "dmitrybugakov (via GitHub)" <gi...@apache.org>.
dmitrybugakov commented on code in PR #12482:
URL: https://github.com/apache/datafusion/pull/12482#discussion_r1761175453
##########
datafusion/sql/src/unparser/rewrite.rs:
##########
@@ -258,8 +258,36 @@ pub(super) fn subquery_alias_inner_query_and_columns(
(outer_projections.input.as_ref(), columns)
}
-/// Injects column aliases into the projection of a logical plan by wrapping `Expr::Column` expressions
-/// with `Expr::Alias` using the provided list of aliases. Non-column expressions are left unchanged.
+/// Injects column aliases into a subquery's logical plan. The function searches for a `Projection`
+/// within the given plan, which may be wrapped by other operators (e.g., LIMIT, SORT).
+/// If the top-level plan is a `Projection`, it directly injects the column aliases.
+/// Otherwise, it iterates through the plan's children to locate and transform the `Projection`.
+///
+/// Example:
+/// - `SELECT col1, col2 FROM table LIMIT 10` plan with aliases `["alias_1", "some_alias_2"]` will be transformed to
+/// - `SELECT col1 AS alias_1, col2 AS some_alias_2 FROM table LIMIT 10`
+pub(super) fn inject_column_aliases_into_subquery(
+ plan: LogicalPlan,
+ aliases: Vec<Ident>,
+) -> Result<LogicalPlan> {
+ match &plan {
+ LogicalPlan::Projection(inner_p) => Ok(inject_column_aliases(inner_p, aliases)),
+ _ => {
+ // projection is wrapped by other operator (LIMIT, SORT, etc), iterate through the plan to find it
+ plan.map_children(|child| {
+ if let LogicalPlan::Projection(p) = &child {
+ Ok(Transformed::yes(inject_column_aliases(p, aliases.clone())))
Review Comment:
Is it correct that we clone aliases in every recursive call?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve SQLite subquery tables aliasing unparsing [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #12482:
URL: https://github.com/apache/datafusion/pull/12482#issuecomment-2353701964
cc @phillipleblanc
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org