github-actions[bot] commented on code in PR #65129:
URL: https://github.com/apache/doris/pull/65129#discussion_r3541475374


##########
regression-test/suites/query_p0/set_operations/bucket_shuffle_set_operation.groovy:
##########
@@ -95,6 +95,36 @@ suite("bucket_shuffle_set_operation") {
         select id from bucket_shuffle_set_operation2 where id=1
         """)
 
+    // The basic child of a bucket-shuffle set operation can be a join output 
instead of a
+    // direct scan. In that shape the local exchange planned for the basic 
side must still
+    // partition by the storage bucket function: an execution-hash local 
exchange would not
+    // align with the bucket-distributed side and the set operation would 
compute wrong results.
+    checkShapeAndResult("bucket_shuffle_join_as_basic_child", """
+        select a.id from bucket_shuffle_set_operation1 a
+        join bucket_shuffle_set_operation2 b on a.id = b.id
+        intersect
+        select id from bucket_shuffle_set_operation3""")
+
+    // a set operation child can itself be a set operation whose output claims 
a bucket
+    // distribution; the outer set operation must only treat its children as 
bucket-aligned
+    // when they share the same storage layout
+    checkShapeAndResult("bucket_shuffle_nested_set_operation", """
+        select id from bucket_shuffle_set_operation3
+        union all
+        (select a.id from bucket_shuffle_set_operation1 a
+        join bucket_shuffle_set_operation2 b on a.id = b.id
+        intersect
+        select id from bucket_shuffle_set_operation2)""")
+
+    // when local shuffle is disabled entirely, every pipeline runs a single 
task per
+    // instance so the bucket alignment holds naturally and bucket shuffle is 
still allowed
+    sql "set enable_local_shuffle=false"
+    checkShapeAndResult("bucket_shuffle_when_local_shuffle_off", """

Review Comment:
   This covers the new `enable_local_shuffle=false` allowed path, but it still 
leaves the downgrade paths added above untested. `RequestPropertyDeriver` now 
explicitly downgrades both ordinary set operations and `PhysicalUnion` 
parent-hash requests when set-op bucket shuffle is not allowed, and the 
existing review thread showed the union override was an easy place to miss. 
Please add a shape/result case with `enable_local_shuffle_planner=false` or 
`enable_nereids_distribute_planner=false` that exercises the parent-hash 
union/set-op request and asserts it does not choose bucket shuffle while 
preserving the result.



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

Reply via email to