Copilot commented on code in PR #16809:
URL: https://github.com/apache/iotdb/pull/16809#discussion_r2562630494
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/process/window/TableWindowOperator.java:
##########
@@ -294,6 +294,8 @@ private LinkedList<PartitionExecutor> partition(TsBlock
tsBlock) {
partitionExecutors.addLast(partitionExecutor);
partitionStartInCurrentBlock = partitionEndInCurrentBlock;
+ // Cross multiple TsBlock partition ends
Review Comment:
[nitpick] The comment "Cross multiple TsBlock partition ends" is unclear.
Consider revising to "Reset cross-TsBlock tracking when partition completes" or
"Clear cached start index after partition completion" to better explain why
this reset is necessary.
```suggestion
// Reset cross-TsBlock tracking after partition completion
```
##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/execution/operator/process/window/TableWindowOperatorTest.java:
##########
@@ -192,6 +192,56 @@ public void testMixedPartition() {
}
}
+ @Test
+ public void testMixedPartition2() {
+ long[][] timeArray =
+ new long[][] {
+ {1, 2, 3},
+ {4, 5},
+ {6},
+ };
+ String[][] deviceIdArray =
+ new String[][] {
+ {"d1", "d1", "d2"},
+ {"d2", "d3"},
+ {"d3"},
+ };
+ int[][] valueArray =
+ new int[][] {
+ {1, 2, 3},
+ {4, 5},
+ {6},
+ };
+
+ long[] expectColumn1 = new long[] {1, 2, 3, 4, 5, 6};
+ String[] expectColumn2 = new String[] {"d1", "d1", "d2", "d2", "d3", "d3"};
+ int[] expectColumn4 = new int[] {1, 2, 3, 4, 5, 6};
+ long[] expectColumn5 = new long[] {1, 2, 1, 2, 1, 2};
+
+ int count = 0;
+ try (TableWindowOperator windowOperator =
+ genWindowOperator(timeArray, deviceIdArray, valueArray)) {
+ ListenableFuture<?> listenableFuture = windowOperator.isBlocked();
+ listenableFuture.get();
+ while (!windowOperator.isFinished() && windowOperator.hasNext()) {
+ TsBlock tsBlock = windowOperator.next();
+ if (tsBlock != null && !tsBlock.isEmpty()) {
+ for (int i = 0, size = tsBlock.getPositionCount(); i < size; i++,
count++) {
+ assertEquals(expectColumn1[count],
tsBlock.getColumn(0).getLong(i));
+ assertEquals(
+ expectColumn2[count],
+
tsBlock.getColumn(1).getBinary(i).getStringValue(TSFileConfig.STRING_CHARSET));
+ assertEquals(expectColumn4[count], tsBlock.getColumn(2).getInt(i));
+ assertEquals(expectColumn5[count],
tsBlock.getColumn(3).getLong(i));
+ }
+ }
+ }
Review Comment:
The test doesn't verify that the expected number of rows (6) were actually
processed. Add an assertion after the while loop: `assertEquals(6, count);` to
ensure the operator returns exactly the expected number of rows. Without this
check, the test could pass even if some rows are missing.
--
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]
