github-actions[bot] commented on code in PR #65835:
URL: https://github.com/apache/doris/pull/65835#discussion_r3828157694
##########
regression-test/suites/query_p0/set_operations/bucket_shuffle_set_operation.groovy:
##########
@@ -511,4 +511,198 @@ suite("bucket_shuffle_set_operation") {
assertTrue(checked)
}
}
+
+ // A bucket-shuffled UNION ALL feeding an analytic window that partitions
by the union's
+ // distribution key. Both branches have to be placed on local tasks by the
same bucket
+ // function; if one keeps its bucket placement while the other is
re-partitioned by the
+ // execution hash, a partition is split across tasks and the window
returns per-task
+ // partial counts. The cross join branch contributes two rows per id and
is the larger
+ // side, so it becomes the bucket-shuffle basic child and the plain scan
is shuffled onto
+ // it. Every id must therefore see exactly 3 rows.
+ sql "drop table if exists bucket_shuffle_set_operation_win"
+ sql """create table bucket_shuffle_set_operation_win(id int)
+ distributed by hash(id) buckets 10
properties('replication_num'='1')"""
+ sql """insert into bucket_shuffle_set_operation_win select number from
numbers("number" = "40")"""
+ sql "drop table if exists bucket_shuffle_set_operation_win2"
+ sql """create table bucket_shuffle_set_operation_win2(id int)
+ distributed by hash(id) buckets 10
properties('replication_num'='1')"""
+ sql "insert into bucket_shuffle_set_operation_win2 values (1), (2)"
+ // Pin the statistics. The bucket-shuffle basic child is chosen by row
count, so on freshly
+ // loaded tables whose statistics have not been reported yet the optimizer
can pick a different
+ // distribution and the case would stop exercising the bucket-shuffle path
it is meant to cover.
+ sql "analyze table bucket_shuffle_set_operation_win with sync"
+ sql "analyze table bucket_shuffle_set_operation_win2 with sync"
+
+ order_qt_bucket_shuffle_union_analytic_partition """
+ select cnt, count(*) as rows_with_cnt
+ from (
Review Comment:
[P1] Force the serial scan that exposes this regression
This block inherits `force_to_local_shuffle=false` from line 189. With ten
scan ranges and `parallel_pipeline_task_num=5`, `ScanNode.isSerialNode()` is
false on one BE (`10 < 5`) and two BEs (`10 < 10`), so `OlapScanNode` reports
bucket-hash placement instead of the serial-source `NOOP` that the old code
incorrectly re-partitioned by execution hash. The result therefore stays
correct on those supported runner layouts even if the placement bug returns.
Please wrap every placement oracle in `force_to_local_shuffle=true` (and
restore it afterward), as the earlier window block does.
##########
regression-test/suites/query_p0/set_operations/bucket_shuffle_set_operation.groovy:
##########
@@ -511,4 +511,198 @@ suite("bucket_shuffle_set_operation") {
assertTrue(checked)
}
}
+
+ // A bucket-shuffled UNION ALL feeding an analytic window that partitions
by the union's
+ // distribution key. Both branches have to be placed on local tasks by the
same bucket
+ // function; if one keeps its bucket placement while the other is
re-partitioned by the
+ // execution hash, a partition is split across tasks and the window
returns per-task
+ // partial counts. The cross join branch contributes two rows per id and
is the larger
+ // side, so it becomes the bucket-shuffle basic child and the plain scan
is shuffled onto
+ // it. Every id must therefore see exactly 3 rows.
+ sql "drop table if exists bucket_shuffle_set_operation_win"
+ sql """create table bucket_shuffle_set_operation_win(id int)
+ distributed by hash(id) buckets 10
properties('replication_num'='1')"""
+ sql """insert into bucket_shuffle_set_operation_win select number from
numbers("number" = "40")"""
+ sql "drop table if exists bucket_shuffle_set_operation_win2"
+ sql """create table bucket_shuffle_set_operation_win2(id int)
+ distributed by hash(id) buckets 10
properties('replication_num'='1')"""
+ sql "insert into bucket_shuffle_set_operation_win2 values (1), (2)"
+ // Pin the statistics. The bucket-shuffle basic child is chosen by row
count, so on freshly
+ // loaded tables whose statistics have not been reported yet the optimizer
can pick a different
+ // distribution and the case would stop exercising the bucket-shuffle path
it is meant to cover.
+ sql "analyze table bucket_shuffle_set_operation_win with sync"
+ sql "analyze table bucket_shuffle_set_operation_win2 with sync"
+
+ order_qt_bucket_shuffle_union_analytic_partition """
+ select cnt, count(*) as rows_with_cnt
+ from (
+ select count(*) over (partition by id order by id) cnt
+ from (
+ select id from bucket_shuffle_set_operation_win
+ union all
+ select l.id from bucket_shuffle_set_operation_win l
+ cross join bucket_shuffle_set_operation_win2 r
+ ) t
+ ) x
+ group by cnt"""
+
+ //
------------------------------------------------------------------------------------------
+ // Distribution matrix: union / intersect / except crossed with every
distribution the
+ // regulator can pick for a set operation — unaligned, execution hash,
bucket shuffle with the
+ // basic child on either side, and tables in a colocate group. Whichever
distribution is
+ // chosen, the result has to be the same, which is what each case asserts.
+ //
+ // Only results are checked, deliberately. Plan shapes here depend on the
backend count, the
+ // core count and the fuzzy session variables the pipeline injects, so a
golden shape would
+ // report environment differences as failures. The distribution each case
is meant to exercise
Review Comment:
[P2] Assert the physical alternatives this matrix claims to cover
These rows only assert semantic output. `PhysicalUnion` can use `ANY`
children plus one hash enforcer above the union instead of the
parent-hash/bucket alternative and still return `(3,120)`; INTERSECT/EXCEPT
return the same sets under execution or storage hashing, and here `value == id`
also makes the execution-key cases data-equivalent. A cost or eligibility
regression can therefore remove the named left/right, execution, or colocate
coverage without failing this matrix. Please add narrow dynamic `EXPLAIN`
checks for the bucket-shuffle marker and direct/basic versus distributed child
(using the topology-tolerant pattern already present above) rather than a full
golden plan.
--
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]
