Copilot commented on code in PR #17711:
URL: https://github.com/apache/iotdb/pull/17711#discussion_r3261182667
##########
iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/utils/ObjectTypeUtils.java:
##########
@@ -109,7 +111,23 @@ public static Binary replaceRegionIdForObjectBinary(int
newRegionId, Binary orig
ObjectTypeUtils.parseObjectBinaryToSizeIObjectPathPair(originValue);
IObjectPath objectPath = pair.getRight();
try {
- IObjectPath newObjectPath = null;
+ final Path path = objectPath.getPath();
+ final int regionId = Integer.parseInt(path.getName(0).toString());
+ if (regionId == newRegionId) {
+ return originValue;
+ }
+
+ final IObjectPath newObjectPath;
+ if (objectPath instanceof PlainObjectPath) {
+ newObjectPath =
+ new PlainObjectPath(objectPath.toString().replaceFirst(regionId +
"", newRegionId + ""));
+ } else {
+ final String[] subPath = new String[path.getNameCount() - 1];
+ for (int i = 1; i < path.getNameCount(); i++) {
+ subPath[i - 1] = path.getName(i).toString();
+ }
+ newObjectPath = new Base32ObjectPath(Paths.get(newRegionId + "",
subPath));
Review Comment:
`replaceRegionIdForObjectBinary` rewrites `PlainObjectPath` using
`objectPath.toString().replaceFirst(regionId + "", newRegionId + "")`. This
string-based replacement is fragile (it is not anchored to the first path
segment and could accidentally replace digits elsewhere in the path) and
couples correctness to the textual form/separator. Prefer rebuilding the path
using `Path` segments (replace `path.getName(0)` only) and constructing a new
`PlainObjectPath` from the updated `Path` to avoid accidental replacements.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/write/ObjectNode.java:
##########
@@ -308,6 +323,23 @@ private void readContentFromFile(File file, byte[]
contents) throws IOException
}
}
+ public RelationalInsertRowNode genValueInsertRowNode() throws
IllegalPathException {
+ final RelationalInsertRowNode insertRowNode = new
RelationalInsertRowNode(this.getPlanNodeId());
+ insertRowNode.setAligned(true);
+ insertRowNode.setDeviceID(filePath.getDeviceID());
+ insertRowNode.setTargetPath(new
PartialPath(filePath.getDeviceID().getTableName()));
Review Comment:
`genValueInsertRowNode()` builds `targetPath` via `new
PartialPath(filePath.getDeviceID().getTableName())`, which will split on "."
and apply normal path parsing. Table names in the relational model can
legitimately contain dots/back-quotes (e.g., "root.table1" in existing serde
tests), so this can generate an unintended multi-node path or throw
`IllegalPathException`. Construct the `PartialPath` in the same way as other
relational insert nodes (use the non-splitting constructor / `needSplit=false`)
so the table name is treated as a single segment.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/write/RelationalInsertRowsNode.java:
##########
@@ -195,6 +201,36 @@ public List<WritePlanNode> splitByPartition(IAnalysis
analysis) {
return writePlanNodeList;
}
+ private void handleObjectValue(
+ InsertRowNode insertRowNode,
+ TRegionReplicaSet dataRegionReplicaSet,
+ List<WritePlanNode> writePlanNodeList) {
+ for (int i = 0; i < insertRowNode.getDataTypes().length; i++) {
+ if (insertRowNode.getDataTypes()[i] != TSDataType.OBJECT) {
+ continue;
+ }
+ final Object[] values = insertRowNode.getValues();
+ if (values[i] == null) {
+ continue;
+ }
+ final byte[] binary = ((Binary) values[i]).getValues();
+ final ByteBuffer buffer = ByteBuffer.wrap(binary);
+ final boolean isEOF = buffer.get() == 1;
+ final long offset = buffer.getLong();
+ final byte[] content = ReadWriteIOUtils.readBytes(buffer,
buffer.remaining());
+ final IObjectPath relativePath =
Review Comment:
`handleObjectValue` parses the OBJECT column as a piece payload but does not
guard against empty/too-short `Binary` values (e.g., `Binary.EMPTY_VALUE` used
to represent NULLs). Calling `buffer.get()`/`getLong()` on an empty buffer will
throw `BufferUnderflowException` and fail the whole split. Add a `binary ==
null || binary.length == 0` (and ideally minimum-length) check before parsing,
consistent with the tablet splitting path.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/sink/protocol/iotconsensusv2/payload/request/IoTConsensusV2ObjectFilePieceReq.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.pipe.sink.protocol.iotconsensusv2.payload.request;
+
+import org.apache.iotdb.common.rpc.thrift.TConsensusGroupId;
+import
org.apache.iotdb.commons.pipe.sink.payload.iotconsensusv2.request.IoTConsensusV2RequestType;
+import
org.apache.iotdb.commons.pipe.sink.payload.iotconsensusv2.request.IoTConsensusV2RequestVersion;
+import org.apache.iotdb.commons.queryengine.plan.planner.plan.node.PlanNode;
+import org.apache.iotdb.consensus.iotconsensusv2.thrift.TCommitId;
+import
org.apache.iotdb.consensus.iotconsensusv2.thrift.TIoTConsensusV2TransferReq;
+import org.apache.iotdb.db.queryengine.plan.planner.plan.node.write.ObjectNode;
+import org.apache.iotdb.db.storageengine.dataregion.wal.buffer.WALEntry;
+
+import java.nio.ByteBuffer;
+import java.util.Objects;
+
+public class IoTConsensusV2ObjectFilePieceReq extends
TIoTConsensusV2TransferReq {
+
+ private transient ObjectNode objectNode;
+
+ private IoTConsensusV2ObjectFilePieceReq() {
+ // Do nothing
+ }
+
+ public ObjectNode getObjectNode() {
+ return objectNode;
+ }
+
+ /////////////////////////////// Thrift ///////////////////////////////
+
+ public static IoTConsensusV2ObjectFilePieceReq toTIoTConsensusV2TransferReq(
+ final ObjectNode objectNode,
+ final TCommitId commitId,
+ final TConsensusGroupId consensusGroupId,
+ final int thisDataNodeId) {
+ final IoTConsensusV2ObjectFilePieceReq req = new
IoTConsensusV2ObjectFilePieceReq();
+
+ req.objectNode = objectNode;
+ req.commitId = commitId;
+ req.consensusGroupId = consensusGroupId;
+ req.dataNodeId = thisDataNodeId;
+ req.version = IoTConsensusV2RequestVersion.VERSION_1.getVersion();
+ req.type = IoTConsensusV2RequestType.TRANSFER_OBJECT_FILE_PIECE.getType();
+ req.body = objectNode.serialize();
+
+ return req;
+ }
+
+ public static IoTConsensusV2ObjectFilePieceReq
fromTIoTConsensusV2TransferReq(
+ final TIoTConsensusV2TransferReq transferReq) {
+ final IoTConsensusV2ObjectFilePieceReq req = new
IoTConsensusV2ObjectFilePieceReq();
+
+ final ByteBuffer body = transferReq.body.duplicate();
+ final PlanNode planNode = WALEntry.deserializeForConsensus(body);
Review Comment:
`fromTIoTConsensusV2TransferReq` blindly casts the deserialized plan node to
`ObjectNode`. If the request body is corrupted or mismatched, this will throw
`ClassCastException` and can tear down the receiver path. Consider validating
`instanceof ObjectNode` and returning a clear type-error status (similar to the
receiver’s tablet-binary handling) instead of an unchecked cast.
--
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]
