You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "mihailoale-db (via GitHub)" <gi...@apache.org> on 2025/01/13 10:15:23 UTC
[PR] [SPARK-50801] [SQL] Improve `PlanLogger.logPlanResolution` so it shows just unresolved and resolved plans [spark]
mihailoale-db opened a new pull request, #49468:
URL: https://github.com/apache/spark/pull/49468
### What changes were proposed in this pull request?
Improve `PlanLogger.logPlanResolution` so it shows just unresolved and resolved plans.
### Why are the changes needed?
Currently `PlanLogger` dumps `unresolvedPlan.withNewChildren(resolvedPlan.children` and `resolvedPlan` in `logPlanResolution` method. This leads to internal errors because in `withNewChildren` we have `children.size == newChildren.size` check which can fail sometimes (e.g when `UnresolvedRelation` is substituted with `SubqueryAlias(..., child, ...)`).
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing tests with the `ANALYZER_DUAL_RUN_LEGACY_AND_SINGLE_PASS_RESOLVER` value set to `true`.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50801] [SQL] Improve `PlanLogger.logPlanResolution` so it shows just unresolved and resolved plans [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #49468: [SPARK-50801] [SQL] Improve `PlanLogger.logPlanResolution` so it shows just unresolved and resolved plans
URL: https://github.com/apache/spark/pull/49468
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50801] [SQL] Improve `PlanLogger.logPlanResolution` so it shows just unresolved and resolved plans [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #49468:
URL: https://github.com/apache/spark/pull/49468#discussion_r1913067842
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/PlanLogger.scala:
##########
@@ -46,7 +46,7 @@ class PlanLogger extends Logging {
|${MDC(
QUERY_PLAN,
sideBySide(
- unresolvedPlan.withNewChildren(resolvedPlan.children).treeString,
+ unresolvedPlan.treeString,
Review Comment:
The intention is to only show the diff of the current node, not the entire tree. Can you give an example of the bug?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50801] [SQL] Improve `PlanLogger.logPlanResolution` so it shows just unresolved and resolved plans [spark]
Posted by "mihailoale-db (via GitHub)" <gi...@apache.org>.
mihailoale-db commented on PR #49468:
URL: https://github.com/apache/spark/pull/49468#issuecomment-2589442482
CIs passed. We can merge it.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50801] [SQL] Improve `PlanLogger.logPlanResolution` so it shows just unresolved and resolved plans [spark]
Posted by "mihailoale-db (via GitHub)" <gi...@apache.org>.
mihailoale-db commented on code in PR #49468:
URL: https://github.com/apache/spark/pull/49468#discussion_r1914519336
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/PlanLogger.scala:
##########
@@ -46,7 +46,7 @@ class PlanLogger extends Logging {
|${MDC(
QUERY_PLAN,
sideBySide(
- unresolvedPlan.withNewChildren(resolvedPlan.children).treeString,
+ unresolvedPlan.treeString,
Review Comment:
In this approach we show then plan diff in bottom up manner for every unresolved->resolved node. There are some improvements that can be done for the current framework but that's something that we'll do in future iterations.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-50801] [SQL] Improve `PlanLogger.logPlanResolution` so it shows just unresolved and resolved plans [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #49468:
URL: https://github.com/apache/spark/pull/49468#issuecomment-2589451071
thanks, merging to master!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org