jinchengchenghh commented on code in PR #8914:
URL: https://github.com/apache/incubator-gluten/pull/8914#discussion_r1983013290
##########
backends-velox/src/test/scala/org/apache/gluten/execution/GlutenSQLCollectLimitExecSuite.scala:
##########
@@ -138,4 +140,34 @@ class GlutenSQLCollectLimitExecSuite extends
WholeStageTransformerSuite {
assertGlutenOperatorMatch[ColumnarCollectLimitBaseExec](unionDf,
checkMatch = true)
}
+
+ testWithSpecifiedSparkVersion("ColumnarCollectLimitExec - offset test",
Array("3.4", "3.5")) {
Review Comment:
What's the result for spark3.3? Is the result also correct but operator not
matched?
##########
backends-velox/src/test/scala/org/apache/gluten/execution/GlutenSQLCollectLimitExecSuite.scala:
##########
@@ -58,7 +58,7 @@ class GlutenSQLCollectLimitExecSuite extends
WholeStageTransformerSuite {
testWithSpecifiedSparkVersion(
Review Comment:
testWithSpecifiedSparkVersion -> test
##########
backends-velox/src/main/scala/org/apache/gluten/execution/ColumnarCollectLimitExec.scala:
##########
@@ -125,11 +199,20 @@ case class ColumnarCollectLimitExec(
if (childRDD.getNumPartitions == 1) childRDD
else shuffleLimitedPartitions(childRDD)
- processedRDD.mapPartitions(partition => collectLimitedRows(partition,
limit))
+ processedRDD.mapPartitions(
+ partition => {
+ val droppedRows = dropLimitedRows(partition, offset)
+ val adjustedLimit = Math.max(0, limit - offset)
+ collectLimitedRows(droppedRows, adjustedLimit)
Review Comment:
Can we enhance the collectLimitedRows, we can slice the input RowVector from
offset to adjustedLimit?
##########
backends-velox/src/test/scala/org/apache/gluten/execution/GlutenSQLCollectLimitExecSuite.scala:
##########
@@ -67,7 +67,9 @@ class GlutenSQLCollectLimitExecSuite extends
WholeStageTransformerSuite {
assertGlutenOperatorMatch[ColumnarCollectLimitBaseExec](df, checkMatch =
true)
}
- testWithSpecifiedSparkVersion("ColumnarCollectLimitExec - with filter",
Array("3.2", "3.3")) {
+ testWithSpecifiedSparkVersion(
Review Comment:
ditto, so as others
##########
backends-velox/src/test/scala/org/apache/gluten/execution/GlutenSQLCollectLimitExecSuite.scala:
##########
@@ -138,4 +140,34 @@ class GlutenSQLCollectLimitExecSuite extends
WholeStageTransformerSuite {
assertGlutenOperatorMatch[ColumnarCollectLimitBaseExec](unionDf,
checkMatch = true)
}
+
+ testWithSpecifiedSparkVersion("ColumnarCollectLimitExec - offset test",
Array("3.4", "3.5")) {
Review Comment:
If that, please also add the result check for spark3.2 and spark3.3
##########
backends-velox/src/test/scala/org/apache/gluten/execution/GlutenSQLCollectLimitExecSuite.scala:
##########
@@ -138,4 +140,34 @@ class GlutenSQLCollectLimitExecSuite extends
WholeStageTransformerSuite {
assertGlutenOperatorMatch[ColumnarCollectLimitBaseExec](unionDf,
checkMatch = true)
}
+
+ testWithSpecifiedSparkVersion("ColumnarCollectLimitExec - offset test",
Array("3.4", "3.5")) {
Review Comment:
Please add the test to cover more code path, such as limit(12)
--
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]
