JoshRosen commented on code in PR #49212:
URL: https://github.com/apache/spark/pull/49212#discussion_r1908004655
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala:
##########
@@ -97,16 +99,17 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
*/
def references: AttributeSet = _references()
- private val _references = new TransientLazy({
Review Comment:
I just realized that this is removing the only current uses of
`TransientLazy`.
What do you think about removing it in order to avoid developer confusion?
My concern is that a mixup of `TransientlyLazy` vs `TransientBestEffortLazyVal`
might be difficult for a reviewer to spot unless they're intimately familiar
with the subtle differences that led to adding the latter. Also, it's
`private[spark]` and hasn't shipped yet so no users or developers could be
depending on it, so I think it's cleanest to remove it for now to avoid
confusion (we can always bring it back later if a use-case arises). WDYT?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala:
##########
@@ -97,16 +99,17 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
*/
def references: AttributeSet = _references()
- private val _references = new TransientLazy({
Review Comment:
I just realized that this PR is removing the only current uses of
`TransientLazy`.
What do you think about removing it in order to avoid developer confusion?
My concern is that a mixup of `TransientlyLazy` vs `TransientBestEffortLazyVal`
might be difficult for a reviewer to spot unless they're intimately familiar
with the subtle differences that led to adding the latter. Also, it's
`private[spark]` and hasn't shipped yet so no users or developers could be
depending on it, so I think it's cleanest to remove it for now to avoid
confusion (we can always bring it back later if a use-case arises). WDYT?
--
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]
