mihaibudiu commented on code in PR #3839:
URL: https://github.com/apache/calcite/pull/3839#discussion_r1678421551


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -2785,41 +2785,69 @@ public static double power(BigDecimal b0, BigDecimal 
b1) {
     return Math.pow(b0.doubleValue(), b1.doubleValue());
   }
 
-
   // LN, LOG, LOG10, LOG2
 
-  /** SQL {@code LOG(number, number2)} function applied to double values. */
-  public static @Nullable Double log(double number, double number2, int 
nullFlag) {
-    if (nullFlag == 1 && number <= 0) {
+  /**
+   * SQL {@code LOG(number, base)} function applied to double values.
+   *
+   * @param dialectflag This parameter is used to specify the SQL dialect for 
the LOG function.
+   * 0 represents MySQL dialect, 1 represents BigQuery dialect, and 2 
represents PostgreSQL dialect.
+   */
+  public static @Nullable Double log(double number, double base, int 
dialectflag) {
+    if (dialectflag == 0 && number <= 0) {

Review Comment:
   you only compare dialectflag with 0.
   So this flag should be named something like "nonPositiveIsNull".
   The JavaDoc should say: "if true return null for non-positive values"



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -4202,67 +4206,46 @@ private static class LogicalNotImplementor extends 
AbstractRexCallImplementor {
    * appropriate base (i.e. base e for LN).
    */
   private static class LogImplementor extends AbstractRexCallImplementor {
-    LogImplementor() {
+    private final SqlLibrary library;
+    LogImplementor(SqlLibrary library) {
       super("log", NullPolicy.STRICT, true);
+      this.library = library;
     }
 
     @Override Expression implementSafe(final RexToLixTranslator translator,
         final RexCall call, final List<Expression> argValueList) {
-      return Expressions.call(BuiltInMethod.LOG.method, args(call, 
argValueList));
+      int flag = library == SqlLibrary.MYSQL ? 0 : library == 
SqlLibrary.BIG_QUERY ? 1 : 2;
+      return Expressions.call(BuiltInMethod.LOG.method, args(call, 
argValueList, flag));
     }
 
     private static List<Expression> args(RexCall call,
-        List<Expression> argValueList) {
-      Expression operand0 = argValueList.get(0);
-      final Expressions.FluentList<Expression> list = 
Expressions.list(operand0);
-      switch (call.getOperator().getName()) {
-      case "LOG":
-        if (argValueList.size() == 2) {
-          return 
list.append(argValueList.get(1)).append(Expressions.constant(0));
-        }
-        // fall through
-      case "LN":
-        return 
list.append(Expressions.constant(Math.exp(1))).append(Expressions.constant(0));
-      case "LOG10":
-        return 
list.append(Expressions.constant(BigDecimal.TEN)).append(Expressions.constant(0));
-      default:
-        throw new AssertionError("Operator not found: " + call.getOperator());
+        List<Expression> argValueList, int flag) {
+      Pair<Expression, Expression> operands;
+      Expression operand0;
+      Expression operand1;
+      if (argValueList.size() == 1) {
+        operands = flag == 2
+            ? Pair.of(argValueList.get(0), 
Expressions.constant(BigDecimal.TEN))
+            : Pair.of(argValueList.get(0), Expressions.constant(Math.exp(1)));
+      } else {
+        operands = flag == 1
+            ? Pair.of(argValueList.get(0), argValueList.get(1))
+            : Pair.of(argValueList.get(1), argValueList.get(0));
       }
-    }
-  }
-
-  /** Implementor for the {@code LN}, {@code LOG}, {@code LOG2} and {@code 
LOG10} operators
-   *  on Mysql and Spark library
-   *
-   * <p>Handles all logarithm functions using log rules to determine the
-   * appropriate base (i.e. base e for LN).
-   */
-  private static class LogMysqlImplementor extends AbstractRexCallImplementor {
-    LogMysqlImplementor() {
-      super("log", NullPolicy.STRICT, true);
-    }
-
-    @Override Expression implementSafe(final RexToLixTranslator translator,
-        final RexCall call, final List<Expression> argValueList) {
-      return Expressions.call(BuiltInMethod.LOG.method, args(call, 
argValueList));
-    }
-
-    private static List<Expression> args(RexCall call,
-        List<Expression> argValueList) {
-      Expression operand0 = argValueList.get(0);
+      operand0 = operands.left;
+      operand1 = operands.right;
       final Expressions.FluentList<Expression> list = 
Expressions.list(operand0);
       switch (call.getOperator().getName()) {
       case "LOG":
-        if (argValueList.size() == 2) {
-          return 
list.append(argValueList.get(1)).append(Expressions.constant(1));
-        }
-        // fall through
+        return list.append(operand1).append(Expressions.constant(flag));
+          // fail through

Review Comment:
   this actually does not fall through, so you should just delete the comment.



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

Reply via email to