jt2594838 commented on code in PR #18248:
URL: https://github.com/apache/iotdb/pull/18248#discussion_r3612544883
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/TabletDecoder.java:
##########
@@ -129,16 +135,51 @@ public Pair<Object[], ByteBuffer> decodeValues(ByteBuffer
buffer) {
RPCServiceThriftHandlerMetrics.getInstance().recordCompressionSizeTimer(compressedSize);
long startDecodeTime = System.nanoTime();
- Object[] columns = new Object[dataTypes.length];
- for (int i = 0; i < dataTypes.length; i++) {
- columns[i] = decodeColumn(uncompressed, i);
+ Object[] columns;
+ if (allValueColumnsPlain) {
+ columns =
+ QueryDataSetUtils.readTabletValuesFromBuffer(
+ uncompressed, dataTypes, dataTypes.length, rowSize);
+ } else {
+ columns = new Object[dataTypes.length];
+ for (int i = 0; i < dataTypes.length; i++) {
+ columns[i] = decodeColumn(uncompressed, i);
+ }
}
RPCServiceThriftHandlerMetrics.getInstance()
.recordDecodeLatencyTimer(System.nanoTime() - startDecodeTime);
return new Pair<>(columns, uncompressed);
}
+ private static boolean allValueColumnsPlain(
+ TSDataType[] dataTypes, List<TSEncoding> columnEncodings) {
+ for (int i = 0; i < dataTypes.length; i++) {
+ if (columnEncodings.get(i + 1) != TSEncoding.PLAIN ||
!supportsPlainFastPath(dataTypes[i])) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static boolean supportsPlainFastPath(TSDataType dataType) {
+ switch (dataType) {
+ case BOOLEAN:
+ case DATE:
+ case INT32:
+ case TIMESTAMP:
+ case INT64:
+ case FLOAT:
+ case DOUBLE:
+ case STRING:
+ case BLOB:
+ case TEXT:
+ return true;
+ default:
+ return false;
+ }
Review Comment:
Use enhanced-switch and list all branches.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/PathUtils.java:
##########
@@ -131,6 +131,54 @@ public static List<String>
checkIsLegalSingleMeasurementsAndUpdate(List<String>
return res;
}
+ /**
+ * Check and canonicalize single measurements in place. This avoids
allocating another list when
+ * the input is a mutable list created by Thrift.
+ */
+ public static void
checkIsLegalSingleMeasurementsAndUpdateInPlace(List<String> measurements)
+ throws MetadataException {
+ if (measurements == null) {
+ return;
+ }
+ for (int i = 0; i < measurements.size(); i++) {
+ String measurement = measurements.get(i);
+ measurements.set(
+ i,
+ measurement == null || measurement.isEmpty()
+ ? null
+ : checkAndReturnSingleMeasurement(measurement));
+ }
+ }
+
+ /**
+ * Check and canonicalize lists of single measurements in place. Duplicate
measurements in one
+ * request are checked only once.
+ */
+ public static void checkIsLegalSingleMeasurementListsAndUpdateInPlace(
+ List<List<String>> measurementLists) throws MetadataException {
+ if (measurementLists == null) {
+ return;
+ }
+ Map<String, String> checkedMeasurements = new HashMap<>();
Review Comment:
May avoid creating the map when there is only one list.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/PathUtils.java:
##########
@@ -131,6 +131,54 @@ public static List<String>
checkIsLegalSingleMeasurementsAndUpdate(List<String>
return res;
}
+ /**
+ * Check and canonicalize single measurements in place. This avoids
allocating another list when
+ * the input is a mutable list created by Thrift.
+ */
+ public static void
checkIsLegalSingleMeasurementsAndUpdateInPlace(List<String> measurements)
+ throws MetadataException {
+ if (measurements == null) {
+ return;
+ }
+ for (int i = 0; i < measurements.size(); i++) {
+ String measurement = measurements.get(i);
+ measurements.set(
+ i,
+ measurement == null || measurement.isEmpty()
+ ? null
+ : checkAndReturnSingleMeasurement(measurement));
+ }
+ }
+
+ /**
+ * Check and canonicalize lists of single measurements in place. Duplicate
measurements in one
+ * request are checked only once.
+ */
+ public static void checkIsLegalSingleMeasurementListsAndUpdateInPlace(
+ List<List<String>> measurementLists) throws MetadataException {
+ if (measurementLists == null) {
+ return;
+ }
+ Map<String, String> checkedMeasurements = new HashMap<>();
Review Comment:
And may consider if it is beneficial to do this check if each list is from a
different device.
--
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]
