Add LogicalPlan::SubqueryAlias #2172

andygrove · 2022-04-06T14:40:41Z

Which issue does this PR close?

Closes ##2164.

Rationale for this change

I would like the logical plan to better represent aliased relations to help with translation to a different execution engine. Currently, original table names are discarded during SQL planning.

What changes are included in this PR?

Introduces new SubqueryAlias node in LogicalPlan and updates the SQL planner to wrap this around TableScan rather than just use the alias name in the TableScan.

Are there any user-facing changes?

The query plan will look different now.

alamb · 2022-04-06T18:50:38Z

datafusion/core/src/logical_plan/builder.rs

@@ -518,6 +518,18 @@ impl LogicalPlanBuilder {
        })))
    }

+    /// Apply an alias
+    pub fn alias(&self, alias: &str) -> Result<Self> {


Another way that this same aliasing can be represented is a Projection node

So if the input has schema with columns a, b, c

Then to implement alias("foo") you could build a LogicalPlanNode that was Project([a foo.a, b as foo.b, c as foo.c])

There may be some good reason to introduce a new type of LogicalPlanNode too that I don't understand, but I wanted to point out this alternative

Ah, and I see you don't want to do that :) in the description

I wonder if you could use the DFSchema::relation name for this purpose?

I think I need something to very specifically say that a table is being used as an alias. An easier change might be just to add an additional field to the TableScan to record the original table name. I think I will put up a separate PR for that approach.

So to explain why I need this. I want to use DataFusion as a SQL parser and planner but I want to execute the query in a different engine. I can provide a TableProvider so that DataFuision can get the schema for table person and I get a logical plan. When I go to translate that plan to a physical plan, it refers to a table called peeps (the alias) and I have no way to know the actual table name.

This is the Spark plan for this use case:

'Project [*] +- 'SubqueryAlias peeps +- 'UnresolvedRelation [person], [], false

I think I prefer the approach taken in this PR, as it could handle subqueries rather than just table scans (as suggested by the name).

I think this is a cleaner, more generic approach.

mingmwang · 2022-04-07T03:54:43Z

Looks like in DataFusion, the logical Project node already support Alias. It should be similar to SparkSQL's SubqueryAlias.
And we can also use it to represent a NamedStruct


pub struct Projection {
    /// The list of expressions
    pub expr: Vec<Expr>,
    /// The incoming logical plan
    pub input: Arc<LogicalPlan>,
    /// The schema description of the output
    pub schema: DFSchemaRef,
    /// Projection output relation alias
    pub alias: Option<String>,
}

andygrove · 2022-04-10T20:18:40Z

@alamb @Dandandan @mingmwang I have fixed the test failures and renamed AliasedRelation to SubqueryAlias. This is only implemented for TableScan so far but if this approach is acceptable, I will file follow-on issues to use the same approach for projection and union, which both currently have their own alias fields.

Dandandan · 2022-04-10T20:50:20Z

datafusion/core/src/sql/planner.rs

-                            // take alias into account to support `JOIN table1 as table2`
-                            alias
-                                .as_ref()
-                                .map(|a| a.name.value.as_str())


alamb

Seems like an improvement to me -- I think the projection pushdown code might benefit from an additional test, but I also think the existing coverage is likely sufficient.

alamb · 2022-04-11T22:08:20Z

datafusion/core/src/optimizer/projection_push_down.rs

@@ -432,6 +432,34 @@ fn optimize_plan(
                alias: alias.clone(),
            }))
        }
+        LogicalPlan::SubqueryAlias(SubqueryAlias { input, alias, .. }) => {


is it worth a test for projection pushdown specifically?

Good point. Test added.

alamb · 2022-04-11T22:10:47Z

datafusion/core/src/sql/planner.rs

@@ -3458,7 +3458,8 @@ mod tests {
        let expected = "Projection: #person.first_name, #person.id\
        \n  Inner Join: Using #person.id = #person2.id\
        \n    TableScan: person projection=None\
-        \n    TableScan: person2 projection=None";
+        \n    SubqueryAlias: person2\


the appearance of SubqueryAlias in this query is somewhat strange to me as there is no subquery in this SQL (maybe a better name is RelationAlias or something)?

I originally named this AliasedRelation and then changed it to SubqueryAlias because that is what Spark uses. I don't have a strong preference either way.

me neither. Whatever you prefer is fine with me

Ok, let's leave it as is for now since we will make use of it for subqueries (soon, hopefully).

I filed follow-on issues for use this approach for projection and union:

Replace Projection.alias with SubqueryAlias #2212

Replace Union.alias with SubqueryAlias #2213

alamb · 2022-04-11T22:11:34Z

Thanks @andygrove

andygrove added 2 commits April 5, 2022 18:18

Add LogicalPlan::AliasedRelation

d5d19a1

unit test

0ca7b69

andygrove self-assigned this Apr 6, 2022

github-actions bot added ballista datafusion Changes in the datafusion crate labels Apr 6, 2022

andygrove added 2 commits April 6, 2022 08:51

unit test passes

332444b

fix some regressions

fccd8d0

alamb reviewed Apr 6, 2022

View reviewed changes

alamb mentioned this pull request Apr 8, 2022

[Discuss] Add struct Query for datafusion #2181

Closed

jackwener mentioned this pull request Apr 10, 2022

Remove Alias from Expr #1468

Open

andygrove added 2 commits April 10, 2022 14:05

fix regressions

8c60e6a

rename AliasedRelation to SubqueryAlias

8d2d964

andygrove changed the title ~~Add LogicalPlan::AliasedRelation~~ Add LogicalPlan::SubqeryAlias Apr 10, 2022

merge from master

575f2b9

andygrove marked this pull request as ready for review April 10, 2022 20:16

andygrove changed the title ~~Add LogicalPlan::SubqeryAlias~~ Add LogicalPlan::SubqueryAlias Apr 10, 2022

Dandandan reviewed Apr 10, 2022

View reviewed changes

fmt

2716f69

alamb approved these changes Apr 11, 2022

View reviewed changes

This was referenced Apr 12, 2022

Replace Projection.alias with SubqueryAlias #2212

Closed

Replace Union.alias with SubqueryAlias #2213

Closed

Add unit test for projection push down with table alias

1f043b8

Dandandan approved these changes Apr 12, 2022

View reviewed changes

andygrove merged commit ee95d41 into apache:master Apr 12, 2022

andygrove deleted the aliased-relation-wip branch April 12, 2022 19:14

andygrove mentioned this pull request Apr 14, 2022

Introduce AliasedRelation in logical plan #2164

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LogicalPlan::SubqueryAlias #2172

Add LogicalPlan::SubqueryAlias #2172

andygrove commented Apr 6, 2022 •

edited

Loading

alamb Apr 6, 2022

alamb Apr 6, 2022

andygrove Apr 7, 2022

andygrove Apr 7, 2022

Dandandan Apr 10, 2022

mingmwang commented Apr 7, 2022

andygrove commented Apr 10, 2022

Dandandan Apr 10, 2022

alamb left a comment

alamb Apr 11, 2022

andygrove Apr 12, 2022

alamb Apr 11, 2022

andygrove Apr 12, 2022

alamb Apr 12, 2022

andygrove Apr 12, 2022

alamb commented Apr 11, 2022

Add LogicalPlan::SubqueryAlias #2172

Add LogicalPlan::SubqueryAlias #2172

Conversation

andygrove commented Apr 6, 2022 • edited Loading

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mingmwang commented Apr 7, 2022

andygrove commented Apr 10, 2022

Choose a reason for hiding this comment

alamb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alamb commented Apr 11, 2022

andygrove commented Apr 6, 2022 •

edited

Loading