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


##########
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java:
##########
@@ -184,6 +184,38 @@ public void testQuotedStrings() {
             .getIdentifier().getName(), "Martha''s Vineyard");
   }
 
+  @Test
+  public void testExtract() {
+    {
+      // Case 1 -- Year and date format ('2017-06-15')
+      PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery("SELECT 
EXTRACT(YEAR FROM \"2017-06-15\")");

Review Comment:
   Please use single quotes and assert that it is literal (or change it to 
`timestampColumn` without quotes, which is an identifier). We may have half of 
the test cases with literal, and the other half with identifier



##########
pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/ExtractTransformFunctionTest.java:
##########
@@ -0,0 +1,45 @@
+package org.apache.pinot.core.operator.transform.function;
+
+import java.util.function.LongToIntFunction;
+import org.apache.pinot.common.function.scalar.DateTimeFunctions;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.request.context.RequestContextUtils;
+import org.testng.Assert;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class ExtractTransformFunctionTest extends BaseTransformFunctionTest {
+
+  @DataProvider
+  public static Object[][] testCases() {
+    return new Object[][] {
+        {"year", (LongToIntFunction) DateTimeFunctions::year},
+        {"month", (LongToIntFunction) DateTimeFunctions::month},
+        {"day", (LongToIntFunction) DateTimeFunctions::dayOfMonth},
+        {"hour", (LongToIntFunction) DateTimeFunctions::hour},
+        {"minute", (LongToIntFunction) DateTimeFunctions::minute},
+        {"second", (LongToIntFunction) DateTimeFunctions::second},
+        // TODO: Need to add timezone_hour and timezone_minute
+//      "timezone_hour",
+//      "timezone_minute",
+    };
+  }
+  
+  @Test(dataProvider = "testCases")
+  public void testExtractTransformFunction(String field, LongToIntFunction 
expected) {
+    // NOTE: functionality of ExtractTransformFunction is covered in 
ExtractTransformFunctionTest  
+    // SELECT EXTRACT(YEAR FROM '2017-10-10')
+
+    ExpressionContext expression = 
RequestContextUtils.getExpression(String.format("extract(%s FROM %s)", field, 
TIMESTAMP_COLUMN));
+    
+    TransformFunction transformFunction = 
TransformFunctionFactory.get(expression, _dataSourceMap);
+    Assert.assertTrue(transformFunction instanceof ExtractTransformFunction);
+    int[] value = transformFunction.transformToIntValuesSV(_projectionBlock);
+    for(int i=0; i< _projectionBlock.getNumDocs(); ++i) {

Review Comment:
   Please reformat this file with pinot style. Also we enforce using `i++` 
instead of `++i` for consistency in the checkstyle



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