tanclary commented on code in PR #3689:
URL: https://github.com/apache/calcite/pull/3689#discussion_r1496171662


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -2788,6 +2788,18 @@ public static double log(BigDecimal d0, BigDecimal d1) {
     return Math.log(d0.doubleValue()) / Math.log(d1.doubleValue());
   }
 
+  /** SQL {@code LOG2(number)} function applied to double values. */
+  public static double log2(double d0) {
+    return (Double.isInfinite(log(d0, 2)) && d0 == 0.0) ? Double.NaN : log(d0, 
2);
+  }
+
+  /** SQL {@code LOG2(number)} function applied to
+   * BigDecimal and double values. */
+  public static double log2(BigDecimal d0) {
+    return (Double.isInfinite(log(d0, 2)) && d0.doubleValue() == 0.0) ? 
Double.NaN : log(d0, 2);

Review Comment:
   Is `NaN` definitely what we want to return?



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -2764,7 +2764,7 @@ public static double power(BigDecimal b0, BigDecimal b1) {
   }
 
 
-  // LN, LOG, LOG10
+  // LN, LOG, LOG2, LOG10

Review Comment:
   nit: LOG2 goes after LOG10



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6222,6 +6222,49 @@ void checkRegexpExtract(SqlOperatorFixture f0, 
FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6224";>[CALCITE-6224]
+   * Add LOG2 function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      f.setFor(SqlLibraryOperators.LOG2);
+      f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+          isWithin(1.0, 0.000001));
+      f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+          isWithin(2.0, 0.000001));
+      f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+          isWithin(16.0, 0.000001));
+      f.checkScalarApprox("log2(2.0/3)", "DOUBLE NOT NULL",
+          isWithin(-0.5849625007211561, 0.000001));
+      f.checkScalarApprox("log2(4.0/3)", "DOUBLE NOT NULL",
+          isWithin(0.4150374992788435, 0.000001));
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          isWithin(-1.0, 0.000001));
+      f.checkScalarApprox("log2(cast(10e8 as double))", "DOUBLE NOT NULL",
+          isWithin(29.897352853986263, 0.000001));
+      f.checkScalarApprox("log2(cast(10e8 as float))", "DOUBLE NOT NULL",
+          isWithin(29.897352853986263, 0.000001));
+      f.checkScalarApprox("log2(1e+52)", "DOUBLE NOT NULL",

Review Comment:
   Which of these are negative tests?



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6222,6 +6222,49 @@ void checkRegexpExtract(SqlOperatorFixture f0, 
FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6224";>[CALCITE-6224]
+   * Add LOG2 function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      f.setFor(SqlLibraryOperators.LOG2);

Review Comment:
   You can set this outside of the consumer for f0 no?



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -637,6 +638,7 @@ Builder populate() {
       defineMethod(EXP, BuiltInMethod.EXP.method, NullPolicy.STRICT);
       defineMethod(POWER, BuiltInMethod.POWER.method, NullPolicy.STRICT);
       defineMethod(ABS, BuiltInMethod.ABS.method, NullPolicy.STRICT);
+      defineMethod(LOG2, BuiltInMethod.LOG2.method, NullPolicy.STRICT);

Review Comment:
   Should this use the LogImplementor defined below?



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