Copilot commented on code in PR #16595:
URL: https://github.com/apache/pinot/pull/16595#discussion_r2276225282
##########
pinot-core/src/main/java/org/apache/pinot/core/common/datablock/DataBlockBuilder.java:
##########
@@ -258,195 +261,225 @@ private static void serializeColumnData(List<Object[]>
columns, DataSchema dataS
// Single-value column
case INT: {
int nullPlaceholder = (int) storedType.getNullPlaceholder();
- for (int rowId = 0; rowId < numRows; rowId++) {
- Object value = column[rowId];
- if (value == null) {
- nullBitmap.add(rowId);
- fixedSize.putInt(nullPlaceholder);
- } else {
- fixedSize.putInt((int) value);
+ interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> {;
+ for (int rowId = start; rowId < end; rowId++) {
+ Object value = column[rowId];
+ if (value == null) {
+ nullBitmap.add(rowId);
+ fixedSize.putInt(nullPlaceholder);
+ } else {
+ fixedSize.putInt((int) value);
+ }
}
- }
+ });
break;
}
case LONG: {
long nullPlaceholder = (long) storedType.getNullPlaceholder();
- for (int rowId = 0; rowId < numRows; rowId++) {
- Object value = column[rowId];
- if (value == null) {
- nullBitmap.add(rowId);
- fixedSize.putLong(nullPlaceholder);
- } else {
- fixedSize.putLong((long) value);
+ interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> {
+ for (int rowId = start; rowId < end; rowId++) {
+ Object value = column[rowId];
+ if (value == null) {
+ nullBitmap.add(rowId);
+ fixedSize.putLong(nullPlaceholder);
+ } else {
+ fixedSize.putLong((long) value);
+ }
}
- }
+ });
break;
}
case FLOAT: {
float nullPlaceholder = (float) storedType.getNullPlaceholder();
- for (int rowId = 0; rowId < numRows; rowId++) {
- Object value = column[rowId];
- if (value == null) {
- nullBitmap.add(rowId);
- fixedSize.putFloat(nullPlaceholder);
- } else {
- fixedSize.putFloat((float) value);
+ interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> {
+ for (int rowId = start; rowId < end; rowId++) {
+ Object value = column[rowId];
+ if (value == null) {
+ nullBitmap.add(rowId);
+ fixedSize.putFloat(nullPlaceholder);
+ } else {
+ fixedSize.putFloat((float) value);
+ }
}
- }
+ });
break;
}
case DOUBLE: {
double nullPlaceholder = (double) storedType.getNullPlaceholder();
- for (int rowId = 0; rowId < numRows; rowId++) {
- Object value = column[rowId];
- if (value == null) {
- nullBitmap.add(rowId);
- fixedSize.putDouble(nullPlaceholder);
- } else {
- fixedSize.putDouble((double) value);
+ interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> {
+ for (int rowId = start; rowId < end; rowId++) {
+ Object value = column[rowId];
+ if (value == null) {
+ nullBitmap.add(rowId);
+ fixedSize.putDouble(nullPlaceholder);
+ } else {
+ fixedSize.putDouble((double) value);
+ }
}
- }
+ });
break;
}
case BIG_DECIMAL: {
BigDecimal nullPlaceholder = (BigDecimal)
storedType.getNullPlaceholder();
- for (int rowId = 0; rowId < numRows; rowId++) {
- Object value = column[rowId];
- if (value == null) {
- nullBitmap.add(rowId);
- setColumn(fixedSize, varSize, nullPlaceholder);
- } else {
- setColumn(fixedSize, varSize, (BigDecimal) value);
+ interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> {
+ for (int rowId = 0; rowId < numRows; rowId++) {
Review Comment:
The BIG_DECIMAL case still uses the original loop bounds (0 to numRows)
instead of the interruptable loop parameters (start to end), which defeats the
purpose of the interruptable loop.
```suggestion
for (int rowId = start; rowId < end; rowId++) {
```
##########
pinot-core/src/main/java/org/apache/pinot/core/common/datablock/DataBlockBuilder.java:
##########
@@ -258,195 +261,225 @@ private static void serializeColumnData(List<Object[]>
columns, DataSchema dataS
// Single-value column
case INT: {
int nullPlaceholder = (int) storedType.getNullPlaceholder();
- for (int rowId = 0; rowId < numRows; rowId++) {
- Object value = column[rowId];
- if (value == null) {
- nullBitmap.add(rowId);
- fixedSize.putInt(nullPlaceholder);
- } else {
- fixedSize.putInt((int) value);
+ interruptableLoop(0, numRows, interruptableLoopStep, (start, end) -> {;
Review Comment:
There is an extra semicolon after the opening brace of the lambda
expression, which will cause a compilation error.
```suggestion
interruptableLoop(0, numRows, interruptableLoopStep, (start, end) ->
{
```
##########
pinot-core/src/main/java/org/apache/pinot/core/common/datablock/DataBlockBuilder.java:
##########
@@ -622,4 +655,21 @@ private static void setNull(ByteBuffer fixedSize,
PagedPinotOutputStream varSize
fixedSize.putInt(0);
varSize.writeInt(CustomObject.NULL_TYPE_VALUE);
}
+
+ /// Iterate using two loops.
+ /// The outer loop will iterate over a maximum of configurable step rows and
check for the interruption flag,
+ /// calling the inner loop to process the rows without checking the
interruption flag.
Review Comment:
Use standard Javadoc comments (/** */) instead of triple slash comments for
method documentation.
```suggestion
/**
* Iterate using two loops.
* The outer loop will iterate over a maximum of configurable step rows
and check for the interruption flag,
* calling the inner loop to process the rows without checking the
interruption flag.
*/
```
--
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]
