You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2023/07/11 23:54:58 UTC

[pinot] branch master updated: Support for new dataTime format in `DateTimeGranularitySpec` without explicitly setting size (#11057)

This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 437504331e Support for new dataTime format in `DateTimeGranularitySpec` without explicitly setting size (#11057)
437504331e is described below

commit 437504331eba37f315cdd88c570c2cf0ccf234bc
Author: Sanskar Modi <sa...@gmail.com>
AuthorDate: Wed Jul 12 05:24:51 2023 +0530

    Support for new dataTime format in `DateTimeGranularitySpec` without explicitly setting size (#11057)
---
 .../DateTimeConversionTransformFunctionTest.java   |  2 +-
 .../pinot/spi/data/DateTimeGranularitySpec.java    | 37 +++++++++++++++-------
 .../spi/data/DateTimeGranularitySpecTest.java      | 26 +++++++++++++--
 3 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/DateTimeConversionTransformFunctionTest.java b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/DateTimeConversionTransformFunctionTest.java
index c4cae05ef1..5b3e420841 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/DateTimeConversionTransformFunctionTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/DateTimeConversionTransformFunctionTest.java
@@ -66,7 +66,7 @@ public class DateTimeConversionTransformFunctionTest extends BaseTransformFuncti
         }, new Object[]{"dateTimeConvert(5,'1:MILLISECONDS:EPOCH','1:MINUTES:EPOCH','1:MINUTES')"}, new Object[]{
         String.format("dateTimeConvert(%s,'1:MILLISECONDS:EPOCH','1:MINUTES:EPOCH','1:MINUTES')", INT_MV_COLUMN)
     }, new Object[]{
-        String.format("dateTimeConvert(%s,'1:MILLISECONDS:EPOCH','1:MINUTES:EPOCH','MINUTES')", TIME_COLUMN)
+        String.format("dateTimeConvert(%s,'1:MILLISECONDS:EPOCH','1:MINUTES:EPOCH','MINUTES:1')", TIME_COLUMN)
     }, new Object[]{
         String.format("dateTimeConvert(%s,%s,'1:MINUTES:EPOCH','1:MINUTES')", TIME_COLUMN, INT_SV_COLUMN)
     }
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeGranularitySpec.java b/pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeGranularitySpec.java
index 4e60d45dc6..036432ee04 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeGranularitySpec.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/data/DateTimeGranularitySpec.java
@@ -33,13 +33,14 @@ public class DateTimeGranularitySpec {
   private static final char COLON_SEPARATOR = ':';
   private static final int COLON_FORMAT_SIZE_POSITION = 0;
   private static final int COLON_FORMAT_TIME_UNIT_POSITION = 1;
+  private static final int COLON_FORMAT_NUM_TOKENS = 2;
 
   // Pipe format: 'timeUnit|size'
   private static final char PIPE_SEPARATOR = '|';
   private static final int PIPE_FORMAT_TIME_UNIT_POSITION = 0;
   private static final int PIPE_FORMAT_SIZE_POSITION = 1;
-
-  private static final int NUM_TOKENS = 2;
+  private static final int PIPE_FORMAT_MIN_TOKENS = 1;
+  private static final int PIPE_FORMAT_MAX_TOKENS = 2;
 
   private final int _size;
   private final TimeUnit _timeUnit;
@@ -53,38 +54,52 @@ public class DateTimeGranularitySpec {
     char separator;
     int sizePosition;
     int timeUnitPosition;
+    int minTokens;
+    int maxTokens;
 
     if (Character.isDigit(granularity.charAt(0))) {
       // Colon format: 'size:timeUnit'
       separator = COLON_SEPARATOR;
       sizePosition = COLON_FORMAT_SIZE_POSITION;
       timeUnitPosition = COLON_FORMAT_TIME_UNIT_POSITION;
+
+      minTokens = COLON_FORMAT_NUM_TOKENS;
+      maxTokens = COLON_FORMAT_NUM_TOKENS;
     } else {
-      // Pipe format: 'timeUnit|size'
+      // Pipe format: 'timeUnit(|size)'
       separator = PIPE_SEPARATOR;
       sizePosition = PIPE_FORMAT_SIZE_POSITION;
       timeUnitPosition = PIPE_FORMAT_TIME_UNIT_POSITION;
+
+      minTokens = PIPE_FORMAT_MIN_TOKENS;
+      maxTokens = PIPE_FORMAT_MAX_TOKENS;
     }
 
     String[] granularityTokens = StringUtil.split(granularity, separator, 2);
-    Preconditions.checkArgument(granularityTokens.length >= NUM_TOKENS,
-            "Invalid granularity: %s, must be of format 'timeUnit|size' or 'size:timeUnit'", granularity);
+    Preconditions.checkArgument(granularityTokens.length >= minTokens && granularityTokens.length <= maxTokens,
+            "Invalid granularity: %s, must be of format 'timeUnit(|size)' or 'size:timeUnit'", granularity);
 
     try {
-      _size = Integer.parseInt(granularityTokens[sizePosition]);
+      _timeUnit = TimeUnit.valueOf(granularityTokens[timeUnitPosition]);
     } catch (Exception e) {
       throw new IllegalArgumentException(
-          String.format("Invalid size: %s in granularity: %s", granularityTokens[sizePosition], granularity));
+          String.format("Invalid time unit: %s in granularity: %s", granularityTokens[timeUnitPosition],
+              granularity));
+    }
+
+    // New format without explicitly setting size - use default size = 1
+    if (granularityTokens.length == PIPE_FORMAT_MIN_TOKENS) {
+      _size = 1;
+      return;
     }
-    Preconditions.checkArgument(_size > 0, "Invalid size: %s in granularity: %s, must be positive", _size, granularity);
 
     try {
-      _timeUnit = TimeUnit.valueOf(granularityTokens[timeUnitPosition]);
+      _size = Integer.parseInt(granularityTokens[sizePosition]);
     } catch (Exception e) {
       throw new IllegalArgumentException(
-          String.format("Invalid time unit: %s in granularity: %s", granularityTokens[timeUnitPosition],
-              granularity));
+          String.format("Invalid size: %s in granularity: %s", granularityTokens[sizePosition], granularity));
     }
+    Preconditions.checkArgument(_size > 0, "Invalid size: %s in granularity: %s, must be positive", _size, granularity);
   }
 
   /**
diff --git a/pinot-spi/src/test/java/org/apache/pinot/spi/data/DateTimeGranularitySpecTest.java b/pinot-spi/src/test/java/org/apache/pinot/spi/data/DateTimeGranularitySpecTest.java
index 487a2380c0..ed60c5d915 100644
--- a/pinot-spi/src/test/java/org/apache/pinot/spi/data/DateTimeGranularitySpecTest.java
+++ b/pinot-spi/src/test/java/org/apache/pinot/spi/data/DateTimeGranularitySpecTest.java
@@ -62,10 +62,32 @@ public class DateTimeGranularitySpecTest {
     assertEquals(dateTimeGranularitySpec.getTimeUnit(), TimeUnit.MILLISECONDS);
     assertEquals(dateTimeGranularitySpec.granularityToMillis(), 1);
 
+    // New format without explicitly setting size
+    dateTimeGranularitySpec = new DateTimeGranularitySpec("HOURS");
+    assertEquals(dateTimeGranularitySpec.getSize(), 1);
+    assertEquals(dateTimeGranularitySpec.getTimeUnit(), TimeUnit.HOURS);
+    assertEquals(dateTimeGranularitySpec.granularityToMillis(), 3600000);
+
+    dateTimeGranularitySpec = new DateTimeGranularitySpec("MINUTES");
+    assertEquals(dateTimeGranularitySpec.getSize(), 1);
+    assertEquals(dateTimeGranularitySpec.getTimeUnit(), TimeUnit.MINUTES);
+    assertEquals(dateTimeGranularitySpec.granularityToMillis(), 60000);
+
+    dateTimeGranularitySpec = new DateTimeGranularitySpec("MILLISECONDS");
+    assertEquals(dateTimeGranularitySpec.getSize(), 1);
+    assertEquals(dateTimeGranularitySpec.getTimeUnit(), TimeUnit.MILLISECONDS);
+    assertEquals(dateTimeGranularitySpec.granularityToMillis(), 1);
+
+    // IllegalArguments
+    assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec(""));
+    assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("1"));
+    assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("1:1"));
+    assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("1|1"));
+    assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("DAY:DAY"));
+    assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("DAY|DAY"));
     assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("DAY:1"));
     assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("1|DAY"));
-    assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("DAY:DAY"));
-    assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("1:1"));
+    assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("EPOCH|DAY|1"));
     assertThrows(IllegalArgumentException.class, () -> new DateTimeGranularitySpec("1:DAY:EPOCH"));
   }
 }


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