Copilot commented on code in PR #17064:
URL: https://github.com/apache/iotdb/pull/17064#discussion_r2719782071
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/relational/aggregation/LastByAccumulator.java:
##########
@@ -316,273 +352,257 @@ public void addStatistics(Statistics[] statistics) {
@Override
public void reset() {
initResult = false;
+ initNullTimeValue = false;
xIsNull = true;
this.yLastTime = Long.MIN_VALUE;
this.xResult.reset();
}
- // TODO can add last position optimization if last position is null ?
- protected void addIntInput(
- Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
- int positionCount = mask.getSelectedPositionCount();
+ private boolean checkAndUpdateLastTime(boolean isXValueNull, long curTime) {
+ if (!initResult || curTime > yLastTime) {
+ initResult = true;
+ yLastTime = curTime;
- if (mask.isSelectAll()) {
- for (int i = 0; i < positionCount; i++) {
- if (!yColumn.isNull(i)) {
- updateIntLastValue(xColumn, i, timeColumn.getLong(i));
- }
- }
- } else {
- int[] selectedPositions = mask.getSelectedPositions();
- int position;
- for (int i = 0; i < positionCount; i++) {
- position = selectedPositions[i];
- if (!yColumn.isNull(position)) {
- updateIntLastValue(xColumn, position, timeColumn.getLong(position));
- }
+ if (isXValueNull) {
+ xIsNull = true;
+ return false;
+ } else {
+ xIsNull = false;
+ return true;
}
}
+ return false;
}
- protected void updateIntLastValue(Column xColumn, int xIdx, long curTime) {
- if (!initResult || curTime > yLastTime) {
- initResult = true;
- yLastTime = curTime;
- if (xColumn.isNull(xIdx)) {
+ private boolean checkAndUpdateNullTime(boolean isXValueNull) {
+ if (!initResult && !initNullTimeValue) {
+ initNullTimeValue = true;
+
+ if (isXValueNull) {
xIsNull = true;
+ return false;
} else {
xIsNull = false;
- xResult.setInt(xColumn.getInt(xIdx));
+ return true;
}
}
+ return false;
}
- protected void updateIntLastValue(int val, long curTime) {
- if (!initResult || curTime > yLastTime) {
- initResult = true;
- yLastTime = curTime;
- xIsNull = false;
- xResult.setInt(val);
+ // TODO can add last position optimization if last position is null ?
+ protected void addIntInput(
+ Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
+ int selectPositionCount = mask.getSelectedPositionCount();
+
+ boolean isSelectAll = mask.isSelectAll();
+ int[] selectedPositions = isSelectAll ? null : mask.getSelectedPositions();
+
+ for (int i = 0; i < selectPositionCount; i++) {
+ int position = isSelectAll ? i : selectedPositions[i];
+ if (yColumn.isNull(position)) {
+ continue;
+ }
+
+ // Check if the time is null
+ if (!timeColumn.isNull(position)) {
+ // Case A: The order time is not null. Attempt to update the xResult.
+ updateIntLastValue(
+ xColumn.isNull(position), xColumn.getInt(position),
timeColumn.getLong(position));
+ } else {
+ // Case A: The order time is null. Attempt to update the
xNullTimeValue.
+ updateIntNullTimeValue(xColumn.isNull(position),
xColumn.getInt(position));
+ }
+ }
+ }
+
+ protected void updateIntLastValue(boolean isXValueNull, int xValue, long
curTime) {
+ if (checkAndUpdateLastTime(isXValueNull, curTime)) {
+ xResult.setInt(xValue);
+ }
+ }
+
+ protected void updateIntNullTimeValue(boolean isXValueNull, int xValue) {
+ if (checkAndUpdateNullTime(isXValueNull)) {
+ xResult.setInt(xValue);
}
}
protected void addLongInput(
Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
- int positionCount = mask.getSelectedPositionCount();
+ int selectPositionCount = mask.getSelectedPositionCount();
- if (mask.isSelectAll()) {
- for (int i = 0; i < positionCount; i++) {
- if (!yColumn.isNull(i)) {
- updateLongLastValue(xColumn, i, timeColumn.getLong(i));
- }
+ boolean isSelectAll = mask.isSelectAll();
+ int[] selectedPositions = isSelectAll ? null : mask.getSelectedPositions();
+
+ for (int i = 0; i < selectPositionCount; i++) {
+ int position = isSelectAll ? i : selectedPositions[i];
+ if (yColumn.isNull(position)) {
+ continue;
}
- } else {
- int[] selectedPositions = mask.getSelectedPositions();
- int position;
- for (int i = 0; i < positionCount; i++) {
- position = selectedPositions[i];
- if (!yColumn.isNull(position)) {
- updateLongLastValue(xColumn, position, timeColumn.getLong(position));
- }
+
+ // Check if the time is null
+ if (!timeColumn.isNull(position)) {
+ // Case A: The order time is not null. Attempt to update the xResult.
+ updateLongLastValue(
+ xColumn.isNull(position), xColumn.getLong(position),
timeColumn.getLong(position));
+ } else {
+ // Case A: The order time is null. Attempt to update the
xNullTimeValue.
Review Comment:
The comment says "Case A:" for both branches of the if-else statement. The
second branch (line 449-450) should be labeled "Case B:" to distinguish it from
the first branch.
```suggestion
// Case B: The order time is null. Attempt to update the
xNullTimeValue.
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/relational/aggregation/LastByDescAccumulator.java:
##########
@@ -77,173 +77,167 @@ public boolean hasFinalResult() {
@Override
protected void addIntInput(
Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
- int positionCount = mask.getSelectedPositionCount();
-
- if (mask.isSelectAll()) {
- for (int i = 0; i < positionCount; i++) {
- if (!yColumn.isNull(i)) {
- updateIntLastValue(xColumn, i, timeColumn.getLong(i));
- if (canFinishAfterInit) {
- return;
- }
- }
+ int selectPositionCount = mask.getSelectedPositionCount();
+
+ boolean isSelectAll = mask.isSelectAll();
+ int[] selectedPositions = isSelectAll ? null : mask.getSelectedPositions();
+
+ for (int i = 0; i < selectPositionCount; i++) {
+ int position = isSelectAll ? i : selectedPositions[i];
+ if (yColumn.isNull(position)) {
+ continue;
}
- } else {
- int[] selectedPositions = mask.getSelectedPositions();
- int position;
- for (int i = 0; i < positionCount; i++) {
- position = selectedPositions[i];
- if (!yColumn.isNull(position)) {
- updateIntLastValue(xColumn, position, timeColumn.getLong(position));
- if (canFinishAfterInit) {
- return;
- }
+
+ // Check if the time is null
+ if (!timeColumn.isNull(position)) {
+ // Case A: The order time is not null. Attempt to update the xResult.
+ updateIntLastValue(
+ xColumn.isNull(position), xColumn.getInt(position),
timeColumn.getLong(position));
+ if (canFinishAfterInit) {
+ return;
}
+ } else {
+ // Case A: The order time is null. Attempt to update the
xNullTimeValue.
+ updateIntNullTimeValue(xColumn.isNull(position),
xColumn.getInt(position));
}
}
}
@Override
protected void addLongInput(
Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
- int positionCount = mask.getSelectedPositionCount();
-
- if (mask.isSelectAll()) {
- for (int i = 0; i < positionCount; i++) {
- if (!yColumn.isNull(i)) {
- updateLongLastValue(xColumn, i, timeColumn.getLong(i));
- if (canFinishAfterInit) {
- return;
- }
- }
+ int selectPositionCount = mask.getSelectedPositionCount();
+
+ boolean isSelectAll = mask.isSelectAll();
+ int[] selectedPositions = isSelectAll ? null : mask.getSelectedPositions();
+
+ for (int i = 0; i < selectPositionCount; i++) {
+ int position = isSelectAll ? i : selectedPositions[i];
+ if (yColumn.isNull(position)) {
+ continue;
}
- } else {
- int[] selectedPositions = mask.getSelectedPositions();
- int position;
- for (int i = 0; i < positionCount; i++) {
- position = selectedPositions[i];
- if (!yColumn.isNull(position)) {
- updateLongLastValue(xColumn, position, timeColumn.getLong(position));
- if (canFinishAfterInit) {
- return;
- }
+
+ // Check if the time is null
+ if (!timeColumn.isNull(position)) {
+ // Case A: The order time is not null. Attempt to update the xResult.
+ updateLongLastValue(
+ xColumn.isNull(position), xColumn.getLong(position),
timeColumn.getLong(position));
+ if (canFinishAfterInit) {
+ return;
}
+ } else {
+ // Case A: The order time is null. Attempt to update the
xNullTimeValue.
Review Comment:
The comment says "Case A:" for both branches of the if-else statement. The
second branch (line 129-130) should be labeled "Case B:" to distinguish it from
the first branch.
```suggestion
// Case B: The order time is null. Attempt to update the
xNullTimeValue.
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/relational/aggregation/grouped/GroupedLastByAccumulator.java:
##########
@@ -317,50 +349,54 @@ public void evaluateIntermediate(int groupId,
ColumnBuilder columnBuilder) {
columnBuilder instanceof BinaryColumnBuilder,
"intermediate input and output of LAST_BY should be BinaryColumn");
- if (!inits.get(groupId)) {
- columnBuilder.appendNull();
- } else {
+ if (inits.get(groupId) || initNullTimeValues.get(groupId)) {
columnBuilder.writeBinary(new Binary(serializeTimeWithValue(groupId)));
+ return;
}
+ columnBuilder.appendNull();
}
private byte[] serializeTimeWithValue(int groupId) {
boolean xNull = xNulls.get(groupId);
- int length = Long.BYTES + 1 + (xNull ? 0 : calculateValueLength(groupId));
+ int length = Long.BYTES + 2 + (xNull ? 0 : calculateValueLength(groupId));
byte[] bytes = new byte[length];
+ boolean isOrderTimeNull = !inits.get(groupId);
longToBytes(yLastTimes.get(groupId), bytes, 0);
boolToBytes(xNulls.get(groupId), bytes, Long.BYTES);
+ boolToBytes(isOrderTimeNull, bytes, Long.BYTES + 1);
+
if (!xNull) {
+ int valueOffset = Long.BYTES + 2;
switch (xDataType) {
case INT32:
case DATE:
- intToBytes(xIntValues.get(groupId), bytes, Long.BYTES + 1);
+ intToBytes(xIntValues.get(groupId), bytes, valueOffset);
return bytes;
case INT64:
case TIMESTAMP:
- longToBytes(xLongValues.get(groupId), bytes, Long.BYTES + 1);
+ longToBytes(xLongValues.get(groupId), bytes, valueOffset);
return bytes;
case FLOAT:
- floatToBytes(xFloatValues.get(groupId), bytes, Long.BYTES + 1);
+ floatToBytes(xFloatValues.get(groupId), bytes, valueOffset);
return bytes;
case DOUBLE:
- doubleToBytes(xDoubleValues.get(groupId), bytes, Long.BYTES + 1);
+ doubleToBytes(xDoubleValues.get(groupId), bytes, valueOffset);
return bytes;
case TEXT:
case BLOB:
case OBJECT:
case STRING:
byte[] values = xBinaryValues.get(groupId).getValues();
- intToBytes(values.length, bytes, Long.BYTES + 1);
+ intToBytes(values.length, bytes, valueOffset);
System.arraycopy(values, 0, bytes, length - values.length,
values.length);
return bytes;
case BOOLEAN:
- boolToBytes(xBooleanValues.get(groupId), bytes, Long.BYTES + 1);
+ boolToBytes(xBooleanValues.get(groupId), bytes, valueOffset);
return bytes;
default:
throw new UnSupportedDataTypeException(
- String.format("Unsupported data type in LAST_BY Aggregation:
%s", yDataType));
+ String.format("Unsupported data type in FIRST_BY Aggregation:
%s", xDataType));
Review Comment:
The error message references "FIRST_BY Aggregation" but this is in the
GroupedLastByAccumulator class which handles LAST_BY aggregation. The error
message should reference "LAST_BY Aggregation" instead.
```suggestion
String.format("Unsupported data type in LAST_BY Aggregation:
%s", xDataType));
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/relational/aggregation/FirstByAccumulator.java:
##########
@@ -316,308 +352,274 @@ public void addStatistics(Statistics[] statistics) {
@Override
public void reset() {
initResult = false;
+ initNullTimeValue = false;
xIsNull = true;
this.yFirstTime = Long.MAX_VALUE;
this.xResult.reset();
}
- protected void addIntInput(
- Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
- int positionCount = mask.getSelectedPositionCount();
-
- if (mask.isSelectAll()) {
- for (int i = 0; i < positionCount; i++) {
- if (!yColumn.isNull(i)) {
- updateIntFirstValue(xColumn, i, timeColumn.getLong(i));
- if (canFinishAfterInit) {
- return;
- }
- }
- }
- } else {
- int[] selectedPositions = mask.getSelectedPositions();
- int position;
- for (int i = 0; i < positionCount; i++) {
- position = selectedPositions[i];
- if (!yColumn.isNull(position)) {
- updateIntFirstValue(xColumn, position, timeColumn.getLong(position));
- if (canFinishAfterInit) {
- return;
- }
- }
- }
- }
- }
-
- protected void updateIntFirstValue(Column xColumn, int xIdx, long curTime) {
+ private boolean checkAndUpdateFirstTime(boolean isXValueNull, long curTime) {
if (!initResult || curTime < yFirstTime) {
initResult = true;
yFirstTime = curTime;
- if (xColumn.isNull(xIdx)) {
+
+ if (isXValueNull) {
xIsNull = true;
+ return false;
} else {
xIsNull = false;
- xResult.setInt(xColumn.getInt(xIdx));
+ return true;
}
}
+ return false;
}
- protected void updateIntFirstValue(int val, long curTime) {
- if (!initResult || curTime < yFirstTime) {
- initResult = true;
- yFirstTime = curTime;
- xIsNull = false;
- xResult.setInt(val);
+ private boolean checkAndUpdateNullTime(boolean isXValueNull) {
+ if (!initResult && !initNullTimeValue) {
+ initNullTimeValue = true;
+
+ if (isXValueNull) {
+ xIsNull = true;
+ return false;
+ } else {
+ xIsNull = false;
+ return true;
+ }
}
+ return false;
}
- protected void addLongInput(
+ protected void addIntInput(
Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
- int positionCount = mask.getSelectedPositionCount();
-
- if (mask.isSelectAll()) {
- for (int i = 0; i < positionCount; i++) {
- if (!yColumn.isNull(i)) {
- updateLongFirstValue(xColumn, i, timeColumn.getLong(i));
- if (canFinishAfterInit) {
- return;
- }
- }
+ int selectPositionCount = mask.getSelectedPositionCount();
+
+ boolean isSelectAll = mask.isSelectAll();
+ int[] selectedPositions = isSelectAll ? null : mask.getSelectedPositions();
+
+ for (int i = 0; i < selectPositionCount; i++) {
+ int position = isSelectAll ? i : selectedPositions[i];
+ if (yColumn.isNull(position)) {
+ continue;
}
- } else {
- int[] selectedPositions = mask.getSelectedPositions();
- int position;
- for (int i = 0; i < positionCount; i++) {
- position = selectedPositions[i];
- if (!yColumn.isNull(position)) {
- updateLongFirstValue(xColumn, position,
timeColumn.getLong(position));
- if (canFinishAfterInit) {
- return;
- }
+
+ // Check if the time is null
+ if (!timeColumn.isNull(position)) {
+ // Case A: The order time is not null. Attempt to update the xResult.
+ updateIntFirstValue(
+ xColumn.isNull(position), xColumn.getInt(position),
timeColumn.getLong(position));
+ if (canFinishAfterInit) {
+ return;
}
+ } else {
+ // Case A: The order time is null. Attempt to update the
xNullTimeValue.
Review Comment:
The comment says "Case A:" for both branches of the if-else statement. The
second branch (line 414-415) should be labeled "Case B:" to distinguish it from
the first branch.
```suggestion
// Case B: The order time is null. Attempt to update the
xNullTimeValue.
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/relational/aggregation/grouped/GroupedLastByAccumulator.java:
##########
@@ -259,54 +267,78 @@ public void addIntermediate(int[] groupIds, Column
argument) {
byte[] bytes = argument.getBinary(i).getValues();
long curTime = BytesUtils.bytesToLongFromOffset(bytes, Long.BYTES, 0);
int offset = Long.BYTES;
- boolean isXNull = BytesUtils.bytesToBool(bytes, offset);
+ boolean isXValueNull = BytesUtils.bytesToBool(bytes, offset);
+ offset += 1;
+ boolean isOrderTimeNull = BytesUtils.bytesToBool(bytes, offset);
offset += 1;
int groupId = groupIds[i];
- if (isXNull) {
- if (!inits.get(groupId) || curTime > yLastTimes.get(groupId)) {
- inits.set(groupId, true);
- yLastTimes.set(groupId, curTime);
- xNulls.set(groupId, true);
- }
- continue;
- }
-
switch (xDataType) {
case INT32:
case DATE:
- int xIntVal = BytesUtils.bytesToInt(bytes, offset);
- updateIntLastValue(groupId, xIntVal, curTime);
+ int intVal = isXValueNull ? 0 : BytesUtils.bytesToInt(bytes, offset);
+ if (!isOrderTimeNull) {
+ updateIntLastValue(groupId, isXValueNull, intVal, curTime);
+ } else {
+ updateIntNullTimeValue(groupId, isXValueNull, intVal);
+ }
break;
case INT64:
case TIMESTAMP:
- long longVal = BytesUtils.bytesToLongFromOffset(bytes, Long.BYTES,
offset);
- updateLongLastValue(groupId, longVal, curTime);
+ long longVal =
+ isXValueNull ? 0 : BytesUtils.bytesToLongFromOffset(bytes,
Long.BYTES, offset);
+ if (!isOrderTimeNull) {
+ updateLongLastValue(groupId, isXValueNull, longVal, curTime);
+ } else {
+ updateLongNullTimeValue(groupId, isXValueNull, longVal);
+ }
break;
case FLOAT:
- float floatVal = BytesUtils.bytesToFloat(bytes, offset);
- updateFloatLastValue(groupId, floatVal, curTime);
+ float floatVal = isXValueNull ? 0 : BytesUtils.bytesToFloat(bytes,
offset);
+ if (!isOrderTimeNull) {
+ updateFloatLastValue(groupId, isXValueNull, floatVal, curTime);
+ } else {
+ updateFloatNullTimeValue(groupId, isXValueNull, floatVal);
+ }
break;
case DOUBLE:
- double doubleVal = BytesUtils.bytesToDouble(bytes, offset);
- updateDoubleLastValue(groupId, doubleVal, curTime);
+ double doubleVal = isXValueNull ? 0 :
BytesUtils.bytesToDouble(bytes, offset);
+ if (!isOrderTimeNull) {
+ updateDoubleLastValue(groupId, isXValueNull, doubleVal, curTime);
+ } else {
+ updateDoubleNullTimeValue(groupId, isXValueNull, doubleVal);
+ }
break;
case TEXT:
case BLOB:
case OBJECT:
case STRING:
- int length = BytesUtils.bytesToInt(bytes, offset);
- offset += Integer.BYTES;
- Binary binaryVal = new Binary(BytesUtils.subBytes(bytes, offset,
length));
- updateBinaryLastValue(groupId, binaryVal, curTime);
+ Binary binaryVal = null;
+ if (!isXValueNull) {
+ int length = BytesUtils.bytesToInt(bytes, offset);
+ offset += Integer.BYTES;
+ binaryVal = new Binary(BytesUtils.subBytes(bytes, offset, length));
+ }
+ if (!isOrderTimeNull) {
+ updateBinaryLastValue(groupId, isXValueNull, binaryVal, curTime);
+ } else {
+ updateBinaryNullTimeValue(groupId, isXValueNull, binaryVal);
+ }
break;
case BOOLEAN:
- boolean boolVal = BytesUtils.bytesToBool(bytes, offset);
- updateBooleanLastValue(groupId, boolVal, curTime);
+ boolean boolVal = false;
+ if (!isXValueNull) {
+ boolVal = BytesUtils.bytesToBool(bytes, offset);
+ }
+ if (!isOrderTimeNull) {
+ updateBooleanLastValue(groupId, isXValueNull, boolVal, curTime);
+ } else {
+ updateBooleanNullTimeValue(groupId, isXValueNull, boolVal);
+ }
break;
default:
throw new UnSupportedDataTypeException(
- String.format("Unsupported data type in LAST_BY Aggregation:
%s", yDataType));
+ String.format("Unsupported data type in FIRST_BY Aggregation:
%s", xDataType));
Review Comment:
The error message references "FIRST_BY Aggregation" but this is in the
GroupedLastByAccumulator class which handles LAST_BY aggregation. The error
message should reference "LAST_BY Aggregation" instead.
```suggestion
String.format("Unsupported data type in LAST_BY Aggregation:
%s", xDataType));
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/relational/aggregation/LastByDescAccumulator.java:
##########
@@ -77,173 +77,167 @@ public boolean hasFinalResult() {
@Override
protected void addIntInput(
Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
- int positionCount = mask.getSelectedPositionCount();
-
- if (mask.isSelectAll()) {
- for (int i = 0; i < positionCount; i++) {
- if (!yColumn.isNull(i)) {
- updateIntLastValue(xColumn, i, timeColumn.getLong(i));
- if (canFinishAfterInit) {
- return;
- }
- }
+ int selectPositionCount = mask.getSelectedPositionCount();
+
+ boolean isSelectAll = mask.isSelectAll();
+ int[] selectedPositions = isSelectAll ? null : mask.getSelectedPositions();
+
+ for (int i = 0; i < selectPositionCount; i++) {
+ int position = isSelectAll ? i : selectedPositions[i];
+ if (yColumn.isNull(position)) {
+ continue;
}
- } else {
- int[] selectedPositions = mask.getSelectedPositions();
- int position;
- for (int i = 0; i < positionCount; i++) {
- position = selectedPositions[i];
- if (!yColumn.isNull(position)) {
- updateIntLastValue(xColumn, position, timeColumn.getLong(position));
- if (canFinishAfterInit) {
- return;
- }
+
+ // Check if the time is null
+ if (!timeColumn.isNull(position)) {
+ // Case A: The order time is not null. Attempt to update the xResult.
+ updateIntLastValue(
+ xColumn.isNull(position), xColumn.getInt(position),
timeColumn.getLong(position));
+ if (canFinishAfterInit) {
+ return;
}
+ } else {
+ // Case A: The order time is null. Attempt to update the
xNullTimeValue.
+ updateIntNullTimeValue(xColumn.isNull(position),
xColumn.getInt(position));
Review Comment:
The comment says "Case A:" for both branches of the if-else statement. The
second branch (line 100-101) should be labeled "Case B:" to distinguish it from
the first branch.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/relational/aggregation/FirstByDescAccumulator.java:
##########
@@ -42,137 +42,149 @@ public TableAccumulator copy() {
@Override
protected void addIntInput(
Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
- int positionCount = mask.getSelectedPositionCount();
+ int selectPositionCount = mask.getSelectedPositionCount();
- if (mask.isSelectAll()) {
- for (int i = 0; i < positionCount; i++) {
- if (!yColumn.isNull(i)) {
- updateIntFirstValue(xColumn, i, timeColumn.getLong(i));
- }
+ boolean isSelectAll = mask.isSelectAll();
+ int[] selectedPositions = isSelectAll ? null : mask.getSelectedPositions();
+
+ for (int i = 0; i < selectPositionCount; i++) {
+ int position = isSelectAll ? i : selectedPositions[i];
+ if (yColumn.isNull(position)) {
+ continue;
}
- } else {
- int[] selectedPositions = mask.getSelectedPositions();
- int position;
- for (int i = 0; i < positionCount; i++) {
- position = selectedPositions[i];
- if (!yColumn.isNull(position)) {
- updateIntFirstValue(xColumn, position, timeColumn.getLong(position));
- }
+
+ // Check if the time is null
+ if (!timeColumn.isNull(position)) {
+ // Case A: The order time is not null. Attempt to update the xResult.
+ updateIntFirstValue(
+ xColumn.isNull(position), xColumn.getInt(position),
timeColumn.getLong(position));
+ } else {
+ // Case A: The order time is null. Attempt to update the
xNullTimeValue.
Review Comment:
The comment says "Case A:" for both branches of the if-else statement. The
second branch (line 62-63) should be labeled "Case B:" to distinguish it from
the first branch.
```suggestion
// Case B: The order time is null. Attempt to update the
xNullTimeValue.
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/source/relational/aggregation/LastByAccumulator.java:
##########
@@ -316,273 +352,257 @@ public void addStatistics(Statistics[] statistics) {
@Override
public void reset() {
initResult = false;
+ initNullTimeValue = false;
xIsNull = true;
this.yLastTime = Long.MIN_VALUE;
this.xResult.reset();
}
- // TODO can add last position optimization if last position is null ?
- protected void addIntInput(
- Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
- int positionCount = mask.getSelectedPositionCount();
+ private boolean checkAndUpdateLastTime(boolean isXValueNull, long curTime) {
+ if (!initResult || curTime > yLastTime) {
+ initResult = true;
+ yLastTime = curTime;
- if (mask.isSelectAll()) {
- for (int i = 0; i < positionCount; i++) {
- if (!yColumn.isNull(i)) {
- updateIntLastValue(xColumn, i, timeColumn.getLong(i));
- }
- }
- } else {
- int[] selectedPositions = mask.getSelectedPositions();
- int position;
- for (int i = 0; i < positionCount; i++) {
- position = selectedPositions[i];
- if (!yColumn.isNull(position)) {
- updateIntLastValue(xColumn, position, timeColumn.getLong(position));
- }
+ if (isXValueNull) {
+ xIsNull = true;
+ return false;
+ } else {
+ xIsNull = false;
+ return true;
}
}
+ return false;
}
- protected void updateIntLastValue(Column xColumn, int xIdx, long curTime) {
- if (!initResult || curTime > yLastTime) {
- initResult = true;
- yLastTime = curTime;
- if (xColumn.isNull(xIdx)) {
+ private boolean checkAndUpdateNullTime(boolean isXValueNull) {
+ if (!initResult && !initNullTimeValue) {
+ initNullTimeValue = true;
+
+ if (isXValueNull) {
xIsNull = true;
+ return false;
} else {
xIsNull = false;
- xResult.setInt(xColumn.getInt(xIdx));
+ return true;
}
}
+ return false;
}
- protected void updateIntLastValue(int val, long curTime) {
- if (!initResult || curTime > yLastTime) {
- initResult = true;
- yLastTime = curTime;
- xIsNull = false;
- xResult.setInt(val);
+ // TODO can add last position optimization if last position is null ?
+ protected void addIntInput(
+ Column xColumn, Column yColumn, Column timeColumn, AggregationMask mask)
{
+ int selectPositionCount = mask.getSelectedPositionCount();
+
+ boolean isSelectAll = mask.isSelectAll();
+ int[] selectedPositions = isSelectAll ? null : mask.getSelectedPositions();
+
+ for (int i = 0; i < selectPositionCount; i++) {
+ int position = isSelectAll ? i : selectedPositions[i];
+ if (yColumn.isNull(position)) {
+ continue;
+ }
+
+ // Check if the time is null
+ if (!timeColumn.isNull(position)) {
+ // Case A: The order time is not null. Attempt to update the xResult.
+ updateIntLastValue(
+ xColumn.isNull(position), xColumn.getInt(position),
timeColumn.getLong(position));
+ } else {
+ // Case A: The order time is null. Attempt to update the
xNullTimeValue.
Review Comment:
The comment says "Case A:" for both branches of the if-else statement. The
second branch (line 412-413) should be labeled "Case B:" to distinguish it from
the first branch.
```suggestion
// Case B: The order time is null. Attempt to update the
xNullTimeValue.
```
--
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]
