zinking commented on a change in pull request #2155:
URL: https://github.com/apache/spark/pull/2155#discussion_r757216489
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
##########
@@ -33,29 +28,13 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] {
/** Returns a Seq of the children of this node */
def children: Seq[BaseType]
- /**
- * A globally unique id for this specific instance. Not preserved across
copies.
- * Unlike `equals`, `id` can be used to differentiate distinct but
structurally
- * identical branches of a tree.
- */
- val id = TreeNode.nextId()
-
- /**
- * Returns true if other is the same [[catalyst.trees.TreeNode TreeNode]]
instance. Unlike
- * `equals` this function will return false for different instances of
structurally identical
- * trees.
- */
- def sameInstance(other: TreeNode[_]): Boolean = {
- this.id == other.id
- }
-
/**
* Faster version of equality which short-circuits when two treeNodes are
the same instance.
* We don't just override Object.Equals, as doing so prevents the scala
compiler from from
* generating case class `equals` methods
*/
def fastEquals(other: TreeNode[_]): Boolean = {
- sameInstance(other) || this == other
+ this.eq(other) || this == other
Review comment:
@chenghao-intel shouldn't this == other comes first, it will be more
efficient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
