Skip to content

Commit

Permalink
[SPARK-33071][SPARK-33536][SQL][FOLLOW-UP] Rename deniedMetadataKeys …
Browse files Browse the repository at this point in the history
…to nonInheritableMetadataKeys in Alias

### What changes were proposed in this pull request?

This PR is a followup of #30488. This PR proposes to rename `Alias.deniedMetadataKeys` to `Alias.nonInheritableMetadataKeys` to make it less confusing.

### Why are the changes needed?

To make it easier to maintain and read.

### Does this PR introduce _any_ user-facing change?

No. This is rather a code cleanup.

### How was this patch tested?

Ran the unittests written in the previous PR manually. Jenkins and GitHub Actions in this PR should also test them.

Closes #30682 from HyukjinKwon/SPARK-33071-SPARK-33536.

Authored-by: HyukjinKwon <gurwls223@apache.org>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
  • Loading branch information
HyukjinKwon committed Dec 9, 2020
1 parent 9959d49 commit b5399d4
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ trait AliasHelper {
exprId = a.exprId,
qualifier = a.qualifier,
explicitMetadata = Some(a.metadata),
deniedMetadataKeys = a.deniedMetadataKeys)
nonInheritableMetadataKeys = a.nonInheritableMetadataKeys)
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 @@ -143,14 +143,14 @@ abstract class Attribute extends LeafExpression with NamedExpression with NullIn
* fully qualified way. Consider the examples tableName.name, subQueryAlias.name.
* tableName and subQueryAlias are possible qualifiers.
* @param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
* @param deniedMetadataKeys Keys of metadata entries that are supposed to be removed when
* inheriting the metadata from the child.
* @param nonInheritableMetadataKeys Keys of metadata entries that are supposed to be removed when
* inheriting the metadata from the child.
*/
case class Alias(child: Expression, name: String)(
val exprId: ExprId = NamedExpression.newExprId,
val qualifier: Seq[String] = Seq.empty,
val explicitMetadata: Option[Metadata] = None,
val deniedMetadataKeys: Seq[String] = Seq.empty)
val nonInheritableMetadataKeys: Seq[String] = Seq.empty)
extends UnaryExpression with NamedExpression {

// Alias(Generator, xx) need to be transformed into Generate(generator, ...)
Expand All @@ -172,7 +172,7 @@ case class Alias(child: Expression, name: String)(
child match {
case named: NamedExpression =>
val builder = new MetadataBuilder().withMetadata(named.metadata)
deniedMetadataKeys.foreach(builder.remove)
nonInheritableMetadataKeys.foreach(builder.remove)
builder.build()

case _ => Metadata.empty
Expand All @@ -181,7 +181,10 @@ case class Alias(child: Expression, name: String)(
}

def newInstance(): NamedExpression =
Alias(child, name)(qualifier = qualifier, explicitMetadata = explicitMetadata)
Alias(child, name)(
qualifier = qualifier,
explicitMetadata = explicitMetadata,
nonInheritableMetadataKeys = nonInheritableMetadataKeys)

override def toAttribute: Attribute = {
if (resolved) {
Expand All @@ -201,7 +204,7 @@ case class Alias(child: Expression, name: String)(
override def toString: String = s"$child AS $name#${exprId.id}$typeSuffix$delaySuffix"

override protected final def otherCopyArgs: Seq[AnyRef] = {
exprId :: qualifier :: explicitMetadata :: deniedMetadataKeys :: Nil
exprId :: qualifier :: explicitMetadata :: nonInheritableMetadataKeys :: Nil
}

override def hashCode(): Int = {
Expand All @@ -212,7 +215,8 @@ case class Alias(child: Expression, name: String)(
override def equals(other: Any): Boolean = other match {
case a: Alias =>
name == a.name && exprId == a.exprId && child == a.child && qualifier == a.qualifier &&
explicitMetadata == a.explicitMetadata && deniedMetadataKeys == a.deniedMetadataKeys
explicitMetadata == a.explicitMetadata &&
nonInheritableMetadataKeys == a.nonInheritableMetadataKeys
case _ => false
}

Expand Down
9 changes: 5 additions & 4 deletions sql/core/src/main/scala/org/apache/spark/sql/Column.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1164,10 +1164,11 @@ class Column(val expr: Expression) extends Logging {
* @since 2.0.0
*/
def name(alias: String): Column = withExpr {
// SPARK-33536: The Alias is no longer a column reference after converting to an attribute.
// These denied metadata keys are used to strip the column reference related metadata for
// the Alias. So it won't be caught as a column reference in DetectAmbiguousSelfJoin.
Alias(expr, alias)(deniedMetadataKeys = Seq(Dataset.DATASET_ID_KEY, Dataset.COL_POS_KEY))
// SPARK-33536: an alias is no longer a column reference. Therefore,
// we should not inherit the column reference related metadata in an alias
// so that it is not caught as a column reference in DetectAmbiguousSelfJoin.
Alias(expr, alias)(
nonInheritableMetadataKeys = Seq(Dataset.DATASET_ID_KEY, Dataset.COL_POS_KEY))
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,8 +577,8 @@ class ColumnarAlias(child: ColumnarExpression, name: String)(
override val exprId: ExprId = NamedExpression.newExprId,
override val qualifier: Seq[String] = Seq.empty,
override val explicitMetadata: Option[Metadata] = None,
override val deniedMetadataKeys: Seq[String] = Seq.empty)
extends Alias(child, name)(exprId, qualifier, explicitMetadata, deniedMetadataKeys)
override val nonInheritableMetadataKeys: Seq[String] = Seq.empty)
extends Alias(child, name)(exprId, qualifier, explicitMetadata, nonInheritableMetadataKeys)
with ColumnarExpression {

override def columnarEval(batch: ColumnarBatch): Any = child.columnarEval(batch)
Expand Down Expand Up @@ -715,7 +715,7 @@ case class PreRuleReplaceAddWithBrokenVersion() extends Rule[SparkPlan] {
def replaceWithColumnarExpression(exp: Expression): ColumnarExpression = exp match {
case a: Alias =>
new ColumnarAlias(replaceWithColumnarExpression(a.child),
a.name)(a.exprId, a.qualifier, a.explicitMetadata, a.deniedMetadataKeys)
a.name)(a.exprId, a.qualifier, a.explicitMetadata, a.nonInheritableMetadataKeys)
case att: AttributeReference =>
new ColumnarAttributeReference(att.name, att.dataType, att.nullable,
att.metadata)(att.exprId, att.qualifier)
Expand Down

0 comments on commit b5399d4

Please sign in to comment.