sarutak commented on issue #25260: [SPARK-28520][SQL] WholeStageCodegen does not work property for LocalTableScanExec URL: https://github.com/apache/spark/pull/25260#issuecomment-516636379 Thanks @reactormonk for the comment! > There are a few things I'm wondering about, though. Is LocalTableScanExec the only case where we're doing this kind of "if root then don't codegen" cutoff? If there are other cases, it might make more sense to extract a trait and make the insertWholeStageCodegen logic cleaner and more flexible. Yes for now. First I considered adding new trait but I found the case we need to cut off was only `LocaTableScanExec` so I decided to change `insertWholeStageCodegen` like a `SMJ` case in `insertInputAdapter`. But it's worth re-considering to add new trait. I'll try it. > Also, what about the case when the immediate parent of a LocalTableScanExec is a Spark stage boundary, e.g. an Exchange? That case seems to be covered by this PR already (which doesn't insert a WholeStageCodegenExec right above LocalTableScanExec. Is that correct? Yes, this PR cover the case you mentioned. Actually, I added a test case for test that case but I removed in response to the review comment. Do you think it's still good to add a test case for the case?
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
