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


##########
regression-test/suites/query_p0/explain/test_global_partition_topn_plan.groovy:
##########
@@ -72,4 +72,59 @@ suite("test_global_partition_topn_plan") {
         contains"PhysicalPartitionTopN"
         contains"PhysicalQuickSort"
     }
+
+    explain {
+        sql """select c2, c3, rk from (
+                select c2, c3, rank() over (partition by c2 order by c2, c3) 
as rk
+                from test_global_partition_topn_plan
+            ) tmp where rk <= 1"""
+        check { String explainStr ->
+            def lines = explainStr.readLines()
+            def partitionTopNBlocks = []
+            lines.eachWithIndex { line, index ->
+                if (line.contains("VPartitionTopN")) {
+                    partitionTopNBlocks.add(lines.subList(index, 
Math.min(index + 10, lines.size())).join("\n"))
+                }
+            }
+            assertTrue(!partitionTopNBlocks.isEmpty(), explainStr)
+            partitionTopNBlocks.each { block ->
+                assertTrue(block.find("partition by: c2\\[#\\d+\\]") != null, 
block)
+                assertTrue(block.find("order by: c3\\[#\\d+\\] ASC") != null, 
block)
+                assertTrue(block.find("order by: c2\\[#\\d+\\] ASC") == null, 
block)
+            }
+        }
+    }
+
+    sql "SET global_partition_topn_threshold=2"
+    explain {
+        sql """shape plan select rn from (
+                select row_number() over (partition by c2 order by c2, c3) as 
rn
+                from test_global_partition_topn_plan
+            ) tmp where rn <= 100"""
+        contains"PhysicalPartitionTopN"
+        notContains"PhysicalQuickSort"
+    }
+
+    explain {
+        sql """select * from (
+                select l.c2 as lc2, r.c2 as rc2,
+                       row_number() over (partition by l.c2 order by r.c2) as 
rn
+                from test_global_partition_topn_plan l
+                join test_global_partition_topn_plan r on l.c1 = r.c1
+            ) tmp where rn <= 1"""
+        check { String explainStr ->
+            def lines = explainStr.readLines()
+            def partitionTopNBlocks = []
+            lines.eachWithIndex { line, index ->
+                if (line.contains("VPartitionTopN")) {
+                    partitionTopNBlocks.add(lines.subList(index, 
Math.min(index + 10, lines.size())).join("\n"))
+                }
+            }
+            assertTrue(!partitionTopNBlocks.isEmpty(), explainStr)
+            partitionTopNBlocks.each { block ->
+                assertTrue(block.find("partition by: c2\\[#\\d+\\]") != null, 
block)
+                assertTrue(block.find("order by: c2\\[#\\d+\\] ASC") != null, 
block)

Review Comment:
   This self-join check still allows a false-pass for the case it is meant to 
protect. The query relies on `l.c2` and `r.c2` being different slots even 
though both explain as `c2[#...]`, but these assertions only check that some 
partition line and some order line use the label `c2`. A regression that 
accidentally ordered by the left/partition slot would still print `order by: 
c2[#...] ASC` and pass. Please extract the `#id` from the partition and order 
lines and assert they differ, or otherwise prove the order key is the 
right-side `r.c2` slot.



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