ahshahid commented on code in PR #45446:
URL: https://github.com/apache/spark/pull/45446#discussion_r1524029007
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ColumnResolutionHelper.scala:
##########
@@ -477,6 +482,57 @@ trait ColumnResolutionHelper extends Logging with
DataTypeErrorsBase {
assert(q.children.length == 1)
q.children.head.output
},
+
+ resolveOnDatasetId = (datasetid: Long, name: String) => {
Review Comment:
> the purpose of #43115 is to use plan id for both Spark Connect and classic
Spark. That said, no more already-resolved `AttributeReference`
Further checking the PR #43115 I see that it is substituting DataSet ID
for plan ID for the attributes. Which is I suppose fine, though redundant,
because metadata already contains the datasetID . However main issue with the
current PR #43115 which makes it untesttable I think is, that for resolution
using LogicalPlan ID , the LogicalPlan should contain the Tag PLAN_ID, which is
not being set as it is not SQLConnect plan.
And I am not that well versed to set the Plan ID for usual Spark
LogicalPlans, for testing purposes.
But I will see if I can reconcile my code with the logic used in
tryResolveDataFrameColumns
--
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]
