AngersZhuuuu commented on code in PR #5418:
URL: https://github.com/apache/kyuubi/pull/5418#discussion_r1360196690


##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala:
##########
@@ -36,6 +38,15 @@ class RuleApplyPermanentViewMarker extends Rule[LogicalPlan] 
{
     plan mapChildren {
       case p: PermanentViewMarker => p
       case permanentView: View if hasResolvedPermanentView(permanentView) =>
+        permanentView.transformAllExpressions {
+          case scalarSubquery: ScalarSubquery =>
+            scalarSubquery.plan.transformDown {
+              case p =>
+                p.setTagValue(PERMANNENT_VIEW_SUBQUERY_TAG, true)
+                p
+            }
+            scalarSubquery
+        }

Review Comment:
   > Do you know what's happening? Tagging the whole sub-tree is a bit 
hypercritical to me
   
   If we only tag ScalarSubquery.plan, after generate executedPlan, may loss 
the top level node, since the `ScalarSubquery.plan` is not optimized.
   <img width="1236" alt="截屏2023-10-16 下午2 56 45" 
src="https://github.com/apache/kyuubi/assets/46485123/a93905ea-8340-4204-8cc5-ddf0844dd487";>
   



##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala:
##########
@@ -36,6 +38,15 @@ class RuleApplyPermanentViewMarker extends Rule[LogicalPlan] 
{
     plan mapChildren {
       case p: PermanentViewMarker => p
       case permanentView: View if hasResolvedPermanentView(permanentView) =>
+        permanentView.transformAllExpressions {
+          case scalarSubquery: ScalarSubquery =>
+            scalarSubquery.plan.transformDown {
+              case p =>
+                p.setTagValue(PERMANNENT_VIEW_SUBQUERY_TAG, true)
+                p
+            }
+            scalarSubquery
+        }

Review Comment:
   > Do you know what's happening? Tagging the whole sub-tree is a bit 
hypercritical to me
   
   If we only tag ScalarSubquery.plan, after generate executedPlan, may loss 
the top level node, since the `ScalarSubquery.plan` is not optimized.
   <img width="1236" alt="截屏2023-10-16 下午2 56 45" 
src="https://github.com/apache/kyuubi/assets/46485123/a93905ea-8340-4204-8cc5-ddf0844dd487";>
   



-- 
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]

Reply via email to