[GitHub] amansinha100 commented on a change in pull request #1610: DRILL-6969: Fix inconsistency of reading MaprDB JSON tables using hive plugin when native reader is enabled

[GitBox <gi...@apache.org>]

amansinha100 commented on a change in pull request #1610: DRILL-6969: Fix inconsistency of reading MaprDB JSON tables using hive plugin when native reader is enabled
URL: https://github.com/apache/drill/pull/1610#discussion_r247220136
 
 

 ##########
 File path: contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/CompareFunctionsProcessor.java
 ##########
 @@ -69,21 +69,52 @@ public Boolean visitUnknown(LogicalExpression e, LogicalExpression valueArg) thr
     return false;
   }
 
+  /**
+   * Converts specified function call to be pushed into maprDB JSON scan.
+   *
+   * @param call function call to be pushed
+   * @return CompareFunctionsProcessor instance which contains converted function call
+   */
   public static CompareFunctionsProcessor process(FunctionCall call) {
+    return processWithEvaluator(call, new CompareFunctionsProcessor(call.getName()));
+  }
+
+  /**
+   * Converts specified function call to be pushed into maprDB JSON scan.
+   * For the case when timestamp value is used, it is converted to UTC timezone
+   * before converting to {@link OTimestamp} instance.
+   *
+   * @param call function call to be pushed
+   * @return CompareFunctionsProcessor instance which contains converted function call
+   */
+  public static CompareFunctionsProcessor processWithTimeZoneOffset(FunctionCall call) {
+    CompareFunctionsProcessor processor = new CompareFunctionsProcessor(call.getName()) {
+      @Override
+      protected boolean visitTimestampExpr(SchemaPath path, TimeStampExpression valueArg) {
+        // converts timestamp value from local time zone to UTC
+        long timeStamp = valueArg.getTimeStamp() - DateUtility.TIMEZONE_OFFSET_MILLIS;
+        this.value = KeyValueBuilder.initFrom(new OTimestamp(timeStamp));
+        this.path = path;
+        return true;
+      }
+    };
+    return processWithEvaluator(call, processor);
+  }
+
+  private static CompareFunctionsProcessor processWithEvaluator(FunctionCall call, CompareFunctionsProcessor evaluator) {
     String functionName = call.getName();
     LogicalExpression nameArg = call.args.get(0);
-    LogicalExpression valueArg = call.args.size() >= 2? call.args.get(1) : null;
-    CompareFunctionsProcessor evaluator = new CompareFunctionsProcessor(functionName);
 
 Review comment:
   The new code modifies the evaluator that is supplied by changing the `functionName` and `success` flag and returns   that instance.  Could this cause side effects ? for example if the caller passed the same evaluator to 2 different methods and was expecting the return to be a new instance ?  The 2 called methods may overwrite each other's state.  

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services