ahshahid commented on PR #56181: URL: https://github.com/apache/spark/pull/56181#issuecomment-4572205290
I will link the prs and jiras tomorrow On Fri, May 29, 2026, 12:44 AM Asif Shahid ***@***.***> wrote: > Thank you Shrirang for review. > If it's a single join, say , then in that particular case, dps fix is > sufficient...but got nested joins, say to level's exchange has child as > join , then it won't. > I commented it out for time being, as this pr's focus is dps. The total > plan assertion is a stronger check , but that of course needs other pr. And > the other pr , I have marked as wip as I want to add some specific unit > tests , so as to test all possible JoinExec... > May be once this pr goes in, i will enable the comment in the check in of > other pr. > > On Fri, May 29, 2026, 12:00 AM Shrirang Mhalgi ***@***.***> > wrote: > >> *shrirangmhalgi* left a comment (apache/spark#56181) >> <https://github.com/apache/spark/pull/56181#issuecomment-4571784278> >> >> The test asserts sameResult only at the BatchScanExec level, but the >> full-plan assertion (totalPlan1.sameResult(totalPlan2)) is commented out >> pending #56182 <https://github.com/apache/spark/pull/56182>. Does this >> mean the DPP canonicalization fix alone isn't sufficient for Exchange reuse >> in practice - it also requires the JoinExec fix? If so, might be worth >> noting that dependency in the PR description so reviewers know the two PRs >> are coupled. >> >> Minor: PruneFileSourcePartitionsSuite.scala has a commented-out import >> (// import org.apache.spark.sql.classic.DataFrame) that looks like leftover >> debug code. >> >> — >> Reply to this email directly, view it on GitHub >> <https://github.com/apache/spark/pull/56181?email_source=notifications&email_token=AC6XG2BSNDRJUQ6Q57HDE2T45EYP5A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJXGE3TQNBSG44KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4571784278>, >> or unsubscribe >> <https://github.com/notifications/unsubscribe-auth/AC6XG2AFIT4OLGRUDUWQKYT45EYP5AVCNFSM6AAAAACZQNHNYOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DKNZRG44DIMRXHA> >> . >> Triage notifications, keep track of coding agent tasks and review pull >> requests on the go with GitHub Mobile for iOS >> <https://github.com/notifications/mobile/ios/AC6XG2BLJLWOHTJ6GRTTDQ345EYP5A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJXGE3TQNBSG44KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> >> and Android >> <https://github.com/notifications/mobile/android/AC6XG2DU7U6MVLDNP5CUOKD45EYP5A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJXGE3TQNBSG44KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. >> Download it today! >> You are receiving this because you authored the thread.Message ID: >> ***@***.***> >> > -- 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]
