Skip to content

Conversation

@ulysses-you
Copy link
Contributor

What changes were proposed in this pull request?

  • Add LogicalQueryStage(_, agg: BaseAggregateExec) check in AQEPropagateEmptyRelation
  • Add LeafNode check in PropagateEmptyRelationBase, so we can eliminate LogicalQueryStage to LocalRelation
  • Unify the applyFunc and commonApplyFunc in PropagateEmptyRelationBase

Why are the changes needed?

The Aggregate in AQE is different with others, the LogicalQueryStage looks like LogicalQueryStage(Aggregate, BaseAggregate). We should handle this case specially.

Logically, if the Aggregate grouping expression is not empty, we can eliminate it safely.

Does this PR introduce any user-facing change?

no

How was this patch tested?

add new test in AdaptiveQueryExecSuite

  • Support propagate empty relation through aggregate
  • Support propagate empty relation through union

@github-actions github-actions bot added the SQL label Jan 10, 2022
@ulysses-you ulysses-you reopened this Jan 10, 2022
@ulysses-you ulysses-you reopened this Jan 10, 2022

val (plan2, adaptivePlan2) = runAdaptiveAndVerifyResult(
"SELECT key, count(*) FROM testData WHERE value = 'no_match' GROUP BY key limit 1")
assert(findTopLevelBaeAggregate(plan1).size == 2)
Copy link

Choose a reason for hiding this comment

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

you mean plan2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you @zinking , yes it should be plan2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW the GA seems broken due to unknown, move to #35149

@ulysses-you ulysses-you deleted the SPARK-35442 branch January 10, 2022 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants