Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1621,11 +1621,13 @@ class Analyzer(
expr.find(_.isInstanceOf[Generator]).isDefined
}

private def hasNestedGenerator(expr: NamedExpression): Boolean = expr match {
case UnresolvedAlias(_: Generator, _) => false
case Alias(_: Generator, _) => false
case MultiAlias(_: Generator, _) => false
case other => hasGenerator(other)
private def hasNestedGenerator(expr: NamedExpression): Boolean = {
CleanupAliases.trimNonTopLevelAliases(expr) match {
Copy link
Contributor

Choose a reason for hiding this comment

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

CleanupAliases.trimNonTopLevelAliases only strips Alias expressions. Should we also handle the other two cases?

Copy link
Author

Choose a reason for hiding this comment

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

Updated to handle the MultiAlias and UnresolvedAlias, and updated the unit test to test all 3.

case UnresolvedAlias(_: Generator, _) => false
Copy link
Member

Choose a reason for hiding this comment

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

If we do not have a valid case here, we should not add it. Here, I think we just need to handle the resolved alias.

Copy link
Author

Choose a reason for hiding this comment

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

hasNestedGenerator already handled UnresolvedAlias. I'll change CleanupAliases back to only handling resolved aliases.

Copy link
Author

Choose a reason for hiding this comment

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

done

case Alias(_: Generator, _) => false
case MultiAlias(_: Generator, _) => false
case other => hasGenerator(other)
}
}

private def trimAlias(expr: NamedExpression): Expression = expr match {
Expand Down Expand Up @@ -1666,24 +1668,26 @@ class Analyzer(
// Holds the resolved generator, if one exists in the project list.
var resolvedGenerator: Generate = null

val newProjectList = projectList.flatMap {
case AliasedGenerator(generator, names, outer) if generator.childrenResolved =>
// It's a sanity check, this should not happen as the previous case will throw
// exception earlier.
assert(resolvedGenerator == null, "More than one generator found in SELECT.")

resolvedGenerator =
Generate(
generator,
unrequiredChildIndex = Nil,
outer = outer,
qualifier = None,
generatorOutput = ResolveGenerate.makeGeneratorOutput(generator, names),
child)

resolvedGenerator.generatorOutput
case other => other :: Nil
}
val newProjectList = projectList
.map(CleanupAliases.trimNonTopLevelAliases(_).asInstanceOf[NamedExpression])
.flatMap {
case AliasedGenerator(generator, names, outer) if generator.childrenResolved =>
// It's a sanity check, this should not happen as the previous case will throw
// exception earlier.
assert(resolvedGenerator == null, "More than one generator found in SELECT.")

resolvedGenerator =
Generate(
generator,
unrequiredChildIndex = Nil,
outer = outer,
qualifier = None,
generatorOutput = ResolveGenerate.makeGeneratorOutput(generator, names),
child)

resolvedGenerator.generatorOutput
case other => other :: Nil
}

if (resolvedGenerator != null) {
Project(newProjectList, resolvedGenerator)
Expand Down Expand Up @@ -2394,6 +2398,7 @@ object CleanupAliases extends Rule[LogicalPlan] {
private def trimAliases(e: Expression): Expression = {
e.transformDown {
case Alias(child, _) => child
case MultiAlias(child, _) => child
}
}

Expand All @@ -2403,6 +2408,8 @@ object CleanupAliases extends Rule[LogicalPlan] {
exprId = a.exprId,
qualifier = a.qualifier,
explicitMetadata = Some(a.metadata))
case a: MultiAlias =>
a.copy(child = trimAliases(a.child))
case other => trimAliases(other)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,4 +575,12 @@ class AnalysisSuite extends AnalysisTest with Matchers {
assertAnalysisSuccess(
Project(Seq(UnresolvedAttribute("temp0.a"), UnresolvedAttribute("temp1.a")), join))
}

test("SPARK-24488 Generator with multiple aliases") {
assertAnalysisSuccess(
listRelation.select(Explode('list).as("first_alias").as("second_alias")))
assertAnalysisSuccess(
listRelation.select(MultiAlias(MultiAlias(
PosExplode('list), Seq("first_pos", "first_val")), Seq("second_pos", "second_val"))))
}
}