Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/6587#issuecomment-115059915
Thank you @marmbrus for your patient to explain this again and again. :-)
My point is essentially what the `AttributeReference` is. The `name`,
`qualifiers` most likely the accessories used by `analyzer`, but the `exprId`
and `metadata` are used more widely in user code.
I am complaining some of the unreasonable implementation in the method
`equals` and `hashCode` of the `AttributeReference`, like in the structurally
testing, should we consider the `qualifiers` as well? as we do compare the
`name` in current code, and why not take the `name` into account in `hashCode`?
etc.
I just wondering where most likely we will write code like
`Set[Expression].contains(expr)`, and what the assumption of the developers in
this code snippet, that's the motive I ignore the `name`in the equality testing.
(Previously, I did change the argument lists as below, however, it seems
lots of existed code impacted, so I change it back and still overriding the
method `equals` and `hashCode`, otherwise, we can remove them too.)
```scala
case class AttributeReference(
name: String,
dataType: DataType,
nullable: Boolean = true,
override val metadata: Metadata = Metadata.empty)(
val exprId: ExprId = NamedExpression.newExprId,
val qualifiers: Seq[String] = Nil) extends Attribute
// V.S.
case class AttributeReference(
val exprId: ExprId = NamedExpression.newExprId,
override val metadata: Metadata = Metadata.empty)(
name: String,
dataType: DataType,
nullable: Boolean = true,
val qualifiers: Seq[String] = Nil
) extends Attribute
```
Thus it's not a big burden for developers (by using the `semanticEquals`),
but it is probably very error-prone / inconvenient, particularly in the
aggregation optimizations, even the catalyst extensions(lots of `TreeNode`
object substitution case).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
