Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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 @@ -1568,11 +1568,32 @@ 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 = {
trimNonTopLevelAliases(expr) match {
case UnresolvedAlias(_: Generator, _) => false

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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)
}
}

def trimNonTopLevelAliases(e: Expression): Expression = e match {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of duplicating the function here, could we just fixing CleanupAliases.trimNonTopLevelAliases

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sure- I didn't want to break any existing functionality, but I can do that instead.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

case a: UnresolvedAlias =>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to handle UnresolvedAlias?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In my use case, no. But I wasn't sure if another use case would care.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't think it'll hurt to handle it.

a.copy(child = trimAliases(a.child))
case a: Alias =>
a.copy(child = trimAliases(a.child))(
exprId = a.exprId,
qualifier = a.qualifier,
explicitMetadata = Some(a.metadata))
case a: MultiAlias =>
a.copy(child = trimAliases(a.child))
case other => trimAliases(other)
}

private def trimAliases(e: Expression): Expression = {
e.transformDown {
case expr: NamedExpression => trimAlias(expr)
}
}

private def trimAlias(expr: NamedExpression): Expression = expr match {
Expand Down Expand Up @@ -1613,24 +1634,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(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
Original file line number Diff line number Diff line change
Expand Up @@ -557,4 +557,15 @@ class AnalysisSuite extends AnalysisTest with Matchers {
SubqueryAlias("tbl", testRelation)))
assertAnalysisError(barrier, Seq("cannot resolve '`tbl.b`'"))
}

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"))))
assertAnalysisSuccess(
listRelation.select(UnresolvedAlias(UnresolvedAlias(
Explode('list), Some(e => "first_alias")), Some(e => "second_alias"))))
}
}