Skip to content

Conversation

@scsmithr
Copy link
Contributor

@scsmithr scsmithr commented Apr 15, 2023

Which issue does this PR close?

Closes #6016.

(Opening this to get early feedback)

Rationale for this change

Simplest way I found for fixing the above issue. I didn't want to try to add an analyzer rule for swapping out references in parent nodes.

What changes are included in this PR?

Changes SubqueryAlias to hold a OwnedTableReference instead of just a string.

Are these changes tested?

Adds sqllogictests.

Are there any user-facing changes?

Yes, this changes the public interface for SubqueryAlias.

@github-actions github-actions bot added logical-expr Logical plan and expressions optimizer Optimizer rules labels Apr 15, 2023
@github-actions github-actions bot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Apr 15, 2023
@Jefffrey
Copy link
Contributor

I did find some previous discussion regarding this for what it's worth: #5615 (comment) @scsmithr

cc @alamb

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 @scsmithr and @Jefffrey -- this looks change looks good to me 👍


/// Apply an alias
pub fn alias(self, alias: impl Into<String>) -> Result<Self> {
pub fn alias(self, alias: impl Into<OwnedTableReference>) -> Result<Self> {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 this change makes a lot of sense to me and is consistent with creating names of other relations (like CreateExternalTable)

https://docs.rs/datafusion-expr/22.0.0/datafusion_expr/logical_plan/struct.CreateExternalTable.html

alias: impl Into<OwnedTableReference>,
) -> Result<Self> {
let alias = alias.into();
let table_ref = TableReference::bare(&alias);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@scsmithr scsmithr marked this pull request as ready for review April 16, 2023 15:50
@scsmithr
Copy link
Contributor Author

CI errors unrelated, see #6028

Otherwise I think this is good to go.

@alamb alamb merged commit 18c0f1c into apache:main Apr 17, 2023
@alamb
Copy link
Contributor

alamb commented Apr 17, 2023

Thanks @scsmithr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate logical-expr Logical plan and expressions optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to query view with a qualified name

5 participants