Skip to content

Support SubqueryAlias wrap filter. - #4013

Closed
jackwener wants to merge 2 commits into
apache:masterfrom
jackwener:subquery_alias
Closed

Support SubqueryAlias wrap filter.#4013
jackwener wants to merge 2 commits into
apache:masterfrom
jackwener:subquery_alias

Conversation

@jackwener

Copy link
Copy Markdown
Member

Which issue does this PR close?

part of #3927

Rationale for this change

After remove eliminate cross join in plan_selection().

Integration test will fail.
SELECT*FROM (
SELECT c2
FROM test t1
EXCEPT
SELECT c2
FROM test t2
WHERE c2 IN (3, 4)
) s
ORDER BY c2

What changes are included in this PR?

Are there any user-facing changes?

@github-actionsgithub-actionsBot added the core Core DataFusion crate label Oct 29, 2022
LogicalPlan::Filter(..) => {
self.create_initial_plan(input, session_state).await
}
_ => Err(DataFusionError::Plan("SubqueryAlias should only wrap TableScan or Filter".to_string()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ideally we should be able to support wrapping any plan in a subquery with alias (this could also simplify some planning code).
But I got some errors trying this before.
#3927

@jackwenerjackwenerOct 29, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I may get your point. You hope to unify alias in the same place
Current, alias can exist two part.

  • SubqueryAlias
  • Projection Alias
    This implementation is different from Spark

Spark Subquery don't include alias.

caseclassSubquery(child: LogicalPlan, correlated: Boolean) extendsOrderPreservingUnaryNode {
overridedefoutput:Seq[Attribute] = child.output
overrideprotecteddefwithNewChildInternal(newChild: LogicalPlan):Subquery=
copy(child = newChild)
}

I think your thought is reasonable. We shouldn't put alias in multiple place.

We have two choices:

  • Just SubqueryAlias can contains alias
  • like spark, Subquery don't contain alias

I also agree with former, which is more suitable for the current situation of datafusion

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This PR is just a tmp solution for #4014.

When I try to fix Integrations test, I add these code to fix it.

@jackwener
jackwener marked this pull request as draft October 30, 2022 10:09
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coreCore DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jackwener@Dandandan