[GitHub] [spark] andygrove commented on pull request #32195: [SPARK-35093] [SQL] AQE now respects supportsColumnar when attempting to reuse exchanges
andygrove commented on pull request #32195: URL: https://github.com/apache/spark/pull/32195#issuecomment-824244265 Thanks, @tgravescs. That is a much simpler change! I have updated the PR. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] andygrove commented on pull request #32195: [SPARK-35093] [SQL] AQE now respects supportsColumnar when attempting to reuse exchanges
andygrove commented on pull request #32195: URL: https://github.com/apache/spark/pull/32195#issuecomment-820898286 I'll take another look at `ReuseExchange` tomorrow. It might be worth considering adding a similar check here, perhaps as a separate PR. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] andygrove commented on pull request #32195: [SPARK-35093] [SQL] AQE now respects supportsColumnar when attempting to reuse exchanges
andygrove commented on pull request #32195: URL: https://github.com/apache/spark/pull/32195#issuecomment-820895987 > Does `ReuseExchange` rule also possibly suffer from the issue? In the rule, Spark also only checks `canonicalized` plans of exchanges to choose reused exchange. This hasn't been a problem for us. We only ran into issues with the AQE exchange reuse logic because of the adaptive nature where query stages are being created during execution, and with Spark making changes after we provide the new query stage it is too late for our plugin to correct the issue. Without AQE, where the `ReuseExchange` rule would be applied (as far as I can see), our plugin would operate on the physical plan after Spark has applied the exchange reuse logic and we would be able to modify the plan as required after that, and before execution, to insert any necessary transitions. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] andygrove commented on pull request #32195: [SPARK-35093] [SQL] AQE now respects supportsColumnar when attempting to reuse exchanges
andygrove commented on pull request #32195: URL: https://github.com/apache/spark/pull/32195#issuecomment-820888554 > Does it mean, if Spark converts reused query stage from columnar to row-based, it will be compatible? Just out of curiosity. @viirya Spark plugins can create either columnar or row-based exchanges as required for compatibility with other parts of the query plan. For example. a columnar `BroadcastHashJoin` would expect both of its child plans to also be columnar, so if Spark replaces one of the child plans with a row-based plan it would not be compatible at execution time. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] andygrove commented on pull request #32195: [SPARK-35093] [SQL] AQE now respects supportsColumnar when attempting to reuse exchanges
andygrove commented on pull request #32195: URL: https://github.com/apache/spark/pull/32195#issuecomment-820887601 > Hi, @andygrove . Could you clarify that this is not a correctness issue in the PR description? It only causes job failures, doesn't it? Yes @dongjoon-hyun that is correct. It would result in an invalid plan that would fail to execute. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] andygrove commented on pull request #32195: [SPARK-35093] [SQL] AQE now respects supportsColumnar when attempting to reuse exchanges
andygrove commented on pull request #32195: URL: https://github.com/apache/spark/pull/32195#issuecomment-820673704 @tgravescs fyi -- 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: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
