Skip to content

Commit 1567d8a

Browse files
HyukjinKwonLorenzo Martini
authored andcommitted
[SPARK-33071][SPARK-33536][SQL][FOLLOW-UP] Rename deniedMetadataKeys to nonInheritableMetadataKeys in Alias
### What changes were proposed in this pull request? This PR is a followup of apache#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 apache#30682 from HyukjinKwon/SPARK-33071-SPARK-33536. Authored-by: HyukjinKwon <gurwls223@apache.org> Signed-off-by: HyukjinKwon <gurwls223@apache.org>
1 parent 84c6883 commit 1567d8a

4 files changed

Lines changed: 20 additions & 15 deletions

File tree

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/AliasHelper.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ trait AliasHelper {
9090
exprId = a.exprId,
9191
qualifier = a.qualifier,
9292
explicitMetadata = Some(a.metadata),
93-
deniedMetadataKeys = a.deniedMetadataKeys)
93+
nonInheritableMetadataKeys = a.nonInheritableMetadataKeys)
9494
case a: MultiAlias =>
9595
a.copy(child = trimAliases(a.child))
9696
case other => trimAliases(other)

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,14 @@ abstract class Attribute extends LeafExpression with NamedExpression with NullIn
142142
* fully qualified way. Consider the examples tableName.name, subQueryAlias.name.
143143
* tableName and subQueryAlias are possible qualifiers.
144144
* @param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
145-
* @param deniedMetadataKeys Keys of metadata entries that are supposed to be removed when
146-
* inheriting the metadata from the child.
145+
* @param nonInheritableMetadataKeys Keys of metadata entries that are supposed to be removed when
146+
* inheriting the metadata from the child.
147147
*/
148148
case class Alias(child: Expression, name: String)(
149149
val exprId: ExprId = NamedExpression.newExprId,
150150
val qualifier: Seq[String] = Seq.empty,
151151
val explicitMetadata: Option[Metadata] = None,
152-
val deniedMetadataKeys: Seq[String] = Seq.empty)
152+
val nonInheritableMetadataKeys: Seq[String] = Seq.empty)
153153
extends UnaryExpression with NamedExpression {
154154

155155
// Alias(Generator, xx) need to be transformed into Generate(generator, ...)
@@ -171,7 +171,7 @@ case class Alias(child: Expression, name: String)(
171171
child match {
172172
case named: NamedExpression =>
173173
val builder = new MetadataBuilder().withMetadata(named.metadata)
174-
deniedMetadataKeys.foreach(builder.remove)
174+
nonInheritableMetadataKeys.foreach(builder.remove)
175175
builder.build()
176176

177177
case _ => Metadata.empty
@@ -180,7 +180,10 @@ case class Alias(child: Expression, name: String)(
180180
}
181181

182182
def newInstance(): NamedExpression =
183-
Alias(child, name)(qualifier = qualifier, explicitMetadata = explicitMetadata)
183+
Alias(child, name)(
184+
qualifier = qualifier,
185+
explicitMetadata = explicitMetadata,
186+
nonInheritableMetadataKeys = nonInheritableMetadataKeys)
184187

185188
override def toAttribute: Attribute = {
186189
if (resolved) {
@@ -200,7 +203,7 @@ case class Alias(child: Expression, name: String)(
200203
override def toString: String = s"$child AS $name#${exprId.id}$typeSuffix$delaySuffix"
201204

202205
override protected final def otherCopyArgs: Seq[AnyRef] = {
203-
exprId :: qualifier :: explicitMetadata :: deniedMetadataKeys :: Nil
206+
exprId :: qualifier :: explicitMetadata :: nonInheritableMetadataKeys :: Nil
204207
}
205208

206209
override def hashCode(): Int = {
@@ -211,7 +214,8 @@ case class Alias(child: Expression, name: String)(
211214
override def equals(other: Any): Boolean = other match {
212215
case a: Alias =>
213216
name == a.name && exprId == a.exprId && child == a.child && qualifier == a.qualifier &&
214-
explicitMetadata == a.explicitMetadata && deniedMetadataKeys == a.deniedMetadataKeys
217+
explicitMetadata == a.explicitMetadata &&
218+
nonInheritableMetadataKeys == a.nonInheritableMetadataKeys
215219
case _ => false
216220
}
217221

sql/core/src/main/scala/org/apache/spark/sql/Column.scala

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,10 +1042,11 @@ class Column(val expr: Expression) extends Logging {
10421042
* @since 2.0.0
10431043
*/
10441044
def name(alias: String): Column = withExpr {
1045-
// SPARK-33536: The Alias is no longer a column reference after converting to an attribute.
1046-
// These denied metadata keys are used to strip the column reference related metadata for
1047-
// the Alias. So it won't be caught as a column reference in DetectAmbiguousSelfJoin.
1048-
Alias(expr, alias)(deniedMetadataKeys = Seq(Dataset.DATASET_ID_KEY, Dataset.COL_POS_KEY))
1045+
// SPARK-33536: an alias is no longer a column reference. Therefore,
1046+
// we should not inherit the column reference related metadata in an alias
1047+
// so that it is not caught as a column reference in DetectAmbiguousSelfJoin.
1048+
Alias(expr, alias)(
1049+
nonInheritableMetadataKeys = Seq(Dataset.DATASET_ID_KEY, Dataset.COL_POS_KEY))
10491050
}
10501051

10511052
/**

sql/core/src/test/scala/org/apache/spark/sql/SparkSessionExtensionSuite.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -600,8 +600,8 @@ class ColumnarAlias(child: ColumnarExpression, name: String)(
600600
override val exprId: ExprId = NamedExpression.newExprId,
601601
override val qualifier: Seq[String] = Seq.empty,
602602
override val explicitMetadata: Option[Metadata] = None,
603-
override val deniedMetadataKeys: Seq[String] = Seq.empty)
604-
extends Alias(child, name)(exprId, qualifier, explicitMetadata, deniedMetadataKeys)
603+
override val nonInheritableMetadataKeys: Seq[String] = Seq.empty)
604+
extends Alias(child, name)(exprId, qualifier, explicitMetadata, nonInheritableMetadataKeys)
605605
with ColumnarExpression {
606606

607607
override def columnarEval(batch: ColumnarBatch): Any = child.columnarEval(batch)
@@ -735,7 +735,7 @@ case class PreRuleReplaceAddWithBrokenVersion() extends Rule[SparkPlan] {
735735
def replaceWithColumnarExpression(exp: Expression): ColumnarExpression = exp match {
736736
case a: Alias =>
737737
new ColumnarAlias(replaceWithColumnarExpression(a.child),
738-
a.name)(a.exprId, a.qualifier, a.explicitMetadata, a.deniedMetadataKeys)
738+
a.name)(a.exprId, a.qualifier, a.explicitMetadata, a.nonInheritableMetadataKeys)
739739
case att: AttributeReference =>
740740
new ColumnarAttributeReference(att.name, att.dataType, att.nullable,
741741
att.metadata)(att.exprId, att.qualifier)

0 commit comments

Comments
 (0)