srowen commented on a change in pull request #29711:
URL: https://github.com/apache/spark/pull/29711#discussion_r487927107



##########
File path: 
sql/core/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q19.sf100/simplified.txt
##########
@@ -13,12 +13,16 @@ TakeOrderedAndProject 
[ext_price,brand,brand_id,i_manufact_id,i_manufact]
                         InputAdapter
                           Exchange [ss_customer_sk] #2
                             WholeStageCodegen (4)
-                              Project 
[i_brand_id,i_brand,i_manufact_id,i_manufact,ss_customer_sk,ss_ext_sales_price,s_zip]
+                              Project 
[ss_customer_sk,ss_ext_sales_price,i_brand_id,i_brand,i_manufact_id,i_manufact,s_zip]

Review comment:
       Would this kind of thing possibly affect the order of columns from a 
`select *` or is that accounted for elsewhere?

##########
File path: 
sql/core/src/test/resources/tpcds-plan-stability/approved-plans-modified/q7.sf100/explain.txt
##########
@@ -10,15 +10,15 @@ TakeOrderedAndProject (34)
                :     :- * Project (17)
                :     :  +- * BroadcastHashJoin Inner BuildRight (16)
                :     :     :- * Project (10)
-               :     :     :  +- * BroadcastHashJoin Inner BuildLeft (9)
-               :     :     :     :- BroadcastExchange (5)
-               :     :     :     :  +- * Project (4)
-               :     :     :     :     +- * Filter (3)
-               :     :     :     :        +- * ColumnarToRow (2)
-               :     :     :     :           +- Scan parquet default.date_dim 
(1)
-               :     :     :     +- * Filter (8)
-               :     :     :        +- * ColumnarToRow (7)
-               :     :     :           +- Scan parquet default.store_sales (6)
+               :     :     :  +- * BroadcastHashJoin Inner BuildRight (9)

Review comment:
       I'm skimming the changes, and this seems non-trivial? but then again I 
am not so sure how to read these plans expertly enough to evaluate. The plan 
below starts by scanning a different table, for example.
   
   We may just have to accept changes like this, if they're equivalent, to make 
sure they do not depend on implementation details of hash maps. But @gatorsmile 
@cloud-fan et al do these look like plausible equivalent plans for example?




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to