Jackie-Jiang commented on code in PR #8625:
URL: https://github.com/apache/pinot/pull/8625#discussion_r864272152


##########
pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java:
##########
@@ -597,6 +599,14 @@ public static Serializable getFormattedValue(Object value, 
ColumnDataType dataTy
           }
         }
         return formattedValue;
+      case TIMESTAMP_ARRAY:
+        long[] timestamps = (long[]) value;
+        length = timestamps.length;
+        formattedValue = new String[length];
+        for (int i = 0; i < length; i++) {
+          formattedValue[i] = new Timestamp(timestamps[i]).toString();

Review Comment:
   This method is for PQL which is already deprecated (about to be removed 
soon). We shouldn't need to fix this part. We may fix the 
`DataSchema.ColumnDataType.format()` to include `BOOLEAN_ARRAY` and 
`TIMESTAMP_ARRAY`



##########
pinot-core/src/test/java/org/apache/pinot/core/query/selection/SelectionOperatorServiceTest.java:
##########
@@ -235,11 +238,11 @@ public void 
testCompatibleRowsRenderSelectionResultsWithoutOrdering() {
     List<Serializable[]> resultRows = selectionResults.getRows();
     Serializable[] expectedRow1 = {
         0L, 1.0, 2.0, 3.0, "4", new long[]{5L}, new double[]{6.0}, new 
double[]{7.0}, new double[]{8.0},
-        new String[]{"9"}, "1020"
+        new String[]{"9"}, "1020", "2022-05-02 14:47:35.0", new 
String[]{"2022-05-02 14:47:35.0"}

Review Comment:
   You may create the expected result using `Timestamp` class instead of 
hard-coded strings



##########
pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java:
##########
@@ -289,6 +289,7 @@ public static DataTable 
getDataTableFromRows(Collection<Object[]> rows, DataSche
             dataTableBuilder.setColumn(i, (int[]) columnValue);
             break;
           case LONG_ARRAY:
+          case TIMESTAMP_ARRAY:

Review Comment:
   We should fix `DataSchema.ColumnDataType.getStoredType()` to include 
`BOOLEAN_ARRAY` and `TIMESTAMP_ARRAY` instead of adding the type here



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to