jt2594838 commented on code in PR #17879:
URL: https://github.com/apache/iotdb/pull/17879#discussion_r3379333840


##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/write/InsertNode.java:
##########
@@ -189,13 +190,16 @@ public int measureColumnCnt() {
   }
 
   public boolean isValidMeasurement(int i) {
-    return measurementSchemas != null
+    return measurements != null

Review Comment:
   How could measurements be null?



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/write/InsertTabletNode.java:
##########
@@ -438,6 +438,16 @@ protected Object[] initTabletValues(int columnSize, int 
rowSize, TSDataType[] da
     return values;
   }
 
+  protected Object[] initTabletValuesForSplit(int columnSize, int rowSize, 
TSDataType[] dataTypes) {
+    Object[] values = initTabletValues(columnSize, rowSize, dataTypes);
+    for (int i = 0; i < values.length; i++) {
+      if (!hasColumnForSplit(i)) {
+        values[i] = null;
+      }
+    }
+    return values;
+  }

Review Comment:
   May init nulls directly in `initTabletValues`, avoid creating unnecessary 
objects



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/MemUtils.java:
##########
@@ -67,14 +67,18 @@ public static long getRecordSize(TSDataType dataType, 
Object value, boolean addi
    * memory before insertion
    */
   public static long getRowRecordSize(List<TSDataType> dataTypes, Object[] 
value) {
-    int emptyRecordCount = 0;
+    return getRowRecordSize(dataTypes, value, null);
+  }
+
+  public static long getRowRecordSize(
+      List<TSDataType> dataTypes, Object[] value, TsTableColumnCategory[] 
columnCategories) {
+    int dataTypeIndex = 0;
     long memSize = 0L;
     for (int i = 0; i < value.length; i++) {
-      if (value[i] == null) {
-        emptyRecordCount++;
+      if (value[i] == null || !isFieldColumn(columnCategories, i)) {
         continue;
       }
-      memSize += getRecordSize(dataTypes.get(i - emptyRecordCount), value[i], 
false);
+      memSize += getRecordSize(dataTypes.get(dataTypeIndex++), value[i], 
false);
     }
     return memSize;
   }

Review Comment:
   dataTypes is null-free?
   If you enforce some restrictions on it now, you should add some comment to 
mention it.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/memtable/AbstractMemTable.java:
##########
@@ -341,6 +337,38 @@ private static int computeTabletNullPointsNumber(
     return nullPointsNumber;
   }
 
+  private static int getValidMeasurementNumber(InsertRowNode insertNode, 
boolean countFieldOnly) {
+    int count = 0;
+    for (int i = 0; i < insertNode.getMeasurements().length; i++) {
+      if (isValidMeasurement(insertNode, i, countFieldOnly)) {
+        count++;
+      }
+    }
+    return count;
+  }
+
+  private static int getValidMeasurementNumber(
+      InsertTabletNode insertNode, boolean countFieldOnly) {
+    int count = 0;
+    for (int i = 0; i < insertNode.getMeasurements().length; i++) {
+      if (isValidMeasurement(insertNode, i, countFieldOnly) && 
insertNode.getColumns()[i] != null) {
+        count++;
+      }
+    }
+    return count;
+  }
+
+  private static boolean isValidMeasurement(
+      InsertNode insertNode, int index, boolean countFieldOnly) {
+    return insertNode.getMeasurements()[index] != null
+        && (!countFieldOnly || isFieldMeasurement(insertNode, index));
+  }
+
+  private static boolean isFieldMeasurement(InsertNode insertNode, int index) {
+    return insertNode.getColumnCategories() == null
+        || insertNode.getColumnCategories()[index] == 
TsTableColumnCategory.FIELD;
+  }

Review Comment:
   May encapsulate these methods into InsertNode to reduce redundancy and 
improve clarity.



-- 
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]

Reply via email to