You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "caicancai (via GitHub)" <gi...@apache.org> on 2024/01/25 11:51:05 UTC

[PR] [CALCITE-6224] Add LOG2 function on SqlStdOperatorTable [calcite]

caicancai opened a new pull request, #3648:
URL: https://github.com/apache/calcite/pull/3648

   https://issues.apache.org/jira/browse/CALCITE-6224


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468413621


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,30 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      f.setFor(SqlLibraryOperators.LOG2);

Review Comment:
   I'm trying to solve it



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function on SqlStdOperatorTable [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1466278149


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -703,15 +705,15 @@ Builder populate2() {
       map.put(FLOOR,
           new FloorImplementor(BuiltInMethod.FLOOR.method,
               BuiltInMethod.UNIX_TIMESTAMP_FLOOR.method,
-            BuiltInMethod.UNIX_DATE_FLOOR.method,
-            BuiltInMethod.CUSTOM_TIMESTAMP_FLOOR.method,
-            BuiltInMethod.CUSTOM_DATE_FLOOR.method));
+              BuiltInMethod.UNIX_DATE_FLOOR.method,

Review Comment:
   I will fix it later



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1480247792


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Yes, you need to adjust/add behavior s.t. when this function is used for these dialects, it follows the proper behavior. 



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479917763


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Hello, thank you very much for your suggestion. Regarding negative test, I think of the following case
   
   - log10(-1): When the parameter in log10 is a negative number
   - log10(null): When the parameters in log10 are empty
   - log10(-`Infinity`): When the parameter in log10 is -`Infinity`
   - log10(0): When the parameter in log10 is 0
   
   I tested based on mysql. Only when log(0) and log10(-1), calcite cannot match the results in mysql. In mysql, both of them return NULL. When log10(0), calcite returns -`Infinity. `log10(-1) returns NaN
   
   ![2024-02-06 22-20-16屏幕截图](https://github.com/apache/calcite/assets/77189278/27b21670-5364-463c-b6dc-d750c46d7cd1)
   
   spark sql and mysql return the same results
   
   ![2024-02-06 22-30-32屏幕截图](https://github.com/apache/calcite/assets/77189278/91173a60-695d-409e-b325-3f91655c6d0d)
   
   



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1470970503


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,29 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG2, VmName.EXPAND);
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL);
+    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)", "DOUBLE NOT NULL",
+        "NaN");
+    f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",

Review Comment:
   @tanclary I specially added a test for this bug, I believe that users and developers can understand



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function on SqlStdOperatorTable [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1910070663

   @tanclary Hello. If you have time, can you help me take a look? You wrote and modified the log function before, thank you.
   


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468438469


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));

Review Comment:
   @XuQianJin-Stars @tanclary 
   You can see that there are retained decimal places.
   
   1.  We need to perform special processing for log2(0) and log(10)
   2.  When converting fractions like 2/3 into sqlnode nodes, an integer value is taken, which also needs further modification.
   
   



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));

Review Comment:
   @XuQianJin-Stars @tanclary 
   You can see that there are retained decimal places.
   1.  We need to perform special processing for log2(0) and log(10)
   2.  When converting fractions like 2/3 into sqlnode nodes, an integer value is taken, which also needs further modification.
   
   



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1913111929

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648) **Quality Gate passed**  
   The SonarCloud Quality Gate passed, but some issues were introduced.
   
   [9 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_coverage&view=list)  
   [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648)
   
   


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1469962583


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,29 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG2, VmName.EXPAND);
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL);
+    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)", "DOUBLE NOT NULL",
+        "NaN");
+    f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",

Review Comment:
   I mean it's definitely a bug but you may want to rewrite your case such that the issue is more clearly stated. I would say stick with a single test case, give the expected results and the actual results. Feel free to tag me on any cases or PRs you file.
   
   I should have found this during my previous LOG work but I didn't. So let me know how I can help.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479917763


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Hello, thank you very much for your suggestion. Regarding negative test, I think of the following case
   
   - log10(-1): When the parameter in log10 is a negative number
   - log10(null): When the parameters in log10 are empty
   - log10(-`Infinity`): When the parameter in log10 is -`Infinity`
   - log10(0): When the parameter in log10 is 0
   
   I tested based on mysql. Only when log(0) and log10(-1), calcite cannot match the results in mysql. In mysql, both of them return NULL. When log10(0), calcite returns -`Infinity. `log10(-1) returns NaN
   
   ![test2](https://github.com/apache/calcite/assets/77189278/c29a69b6-8c4e-4d94-b7c3-148da99f3243)
   
   



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477424593


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   I am still of the belief that even though LOG(0) returning `-Infinity` makes sense mathematically, we should do our best to parity the dialects that support these functions. I am a fan of extending a function's behavior, but not augmenting it.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479917763


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Hello, thank you very much for your suggestion. Regarding negative test, I think of the following case
   
   - log10(-1): When the parameter in log10 is a negative number
   - log10(null): When the parameters in log10 are empty
   - log10(-`Infinity`): When the parameter in log10 is -`Infinity`
   - log10(0): When the parameter in log10 is 0
   
   I tested based on mysql. Only when log(0) and log10(-1), calcite cannot match the results in mysql. In mysql, both of them return NULL. When log10(0), calcite returns -`Infinity. `log10(-1) returns NaN
   
   ![2024-02-06 22-20-16屏幕截图](https://github.com/apache/calcite/assets/77189278/27b21670-5364-463c-b6dc-d750c46d7cd1)
   
   spark sql and mysql return the same results
   
   



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1923439078

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648) **Quality Gate passed**  
   The SonarCloud Quality Gate passed, but some issues were introduced.
   
   [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_coverage&view=list)  
   [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648)
   
   


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468413309


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,30 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      f.setFor(SqlLibraryOperators.LOG2);

Review Comment:
   It seems to be a bug. I recorded it with log(2/3) in a jira because log10 also has the same problem.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468431235


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -2142,6 +2142,18 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
           OperandTypes.NUMERIC_OPTIONAL_NUMERIC,
           SqlFunctionCategory.NUMERIC);
 
+  /** The "LOG2(value)" function.
+   *
+   * @see SqlStdOperatorTable#LN
+   * @see SqlStdOperatorTable#LOG10
+   */
+  @LibraryOperator(libraries = {MYSQL, SPARK})

Review Comment:
   https://www.postgresql.org/docs/current/functions-math.html#FUNCTIONS-MATH-FUNC-TABLE
   The official document does not show it, I think there is no need to add it



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1481028118


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   I probably understand what you mean. What you mean is that if you input types such as string and boolean that are not numbers, an error should be reported.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1925608459

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648) **Quality Gate passed**  
   The SonarCloud Quality Gate passed, but some issues were introduced.
   
   [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_coverage&view=list)  
   [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648)
   
   


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479937474


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   My current idea is to add a judgment to the log function in sqlFunctions. If it is less than or equal to 0, it will return 0. But there is a problem, that is, log10 and log functions are adapted to the current calcite database. So what should we do? To solve this problem, we cannot guarantee that the results returned by the database will be the same.
   In other words, Should I write Sqlfunctions specifically for log2 ?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477705614


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Totally agree, if necessary we can do something similar to the `SUBSTR` functions which I think is implemented per-dialect.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1924901070

   I will fix it


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477247017


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Hello, I seem to have forgotten the simplest truth, that is, there are no fractions in computers, only floating point numbers. I changed 2/3 in the test to 2.0/3, and it turns out that the result is correct. If 2/3 must output the correct result, then this bug does not only exist in the log function, but the pow and abs functions also have similar problems. Regarding log(0) I don't think it's a bug.
   
   Thank you very much for your guidance on these two PRs of my adaptation function. I am sorry for causing you a lot of trouble. If there is anything missing in this PR, please point it out, thank you.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477247017


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
    Hello, I seem to have forgotten the simplest truth, that is, there are no fractions in computers, only float/double point numbers. I changed 2/3 in the test to 2.0/3, and it turns out that the result is correct. If 2/3 must output the correct result, then this bug does not only exist in the log function, but the pow and abs functions also have similar problems. Regarding log(0) I don't think it's a bug.
   
   Thank you very much for your guidance on these two PRs of my adaptation function. I am sorry for causing you a lot of trouble. If there is anything missing in this PR, please point it out, thank you.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477418754


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   I would recommend studying the SQL language, otherwise you will have surprises.
   It's very difficult to contribute to a compiler if you don't understand the language you are compiling.
   In SQL floating point is generally not used. SQL has a family of DECIMAL (or NUMERIC -- same thing) data types, which are very different from floating point. 2.0 is a DECIMAL literal. DECIMAL is not a type, it's a family of types with different scales and precisions. Unfortunately, different dialects of SQL treat DECIMAL in incompatible ways. Floating point is an IEEE standard, but DECIMAL isn't.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468283719


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,29 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG2, VmName.EXPAND);
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL);
+    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)", "DOUBLE NOT NULL",
+        "NaN");
+    f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",

Review Comment:
   I put the test into log10 and had the same problem



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468413621


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,30 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      f.setFor(SqlLibraryOperators.LOG2);

Review Comment:
   I'm trying to solve it but maybe it's not solved by this pr



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1467679733


##########
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, LOG10, LOG2

Review Comment:
   done



##########
site/_docs/reference.md:
##########
@@ -1335,6 +1335,7 @@ comp:
 | MOD(numeric1, numeric2)   | Returns the remainder (modulus) of *numeric1* divided by *numeric2*. The result is negative only if *numeric1* is negative
 | SQRT(numeric)             | Returns the square root of *numeric*
 | LN(numeric)               | Returns the natural logarithm (base *e*) of *numeric*
+| LOG2(numeric)             | Returns the base 2 logarithm of *numeric*

Review Comment:
   done



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477862898


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   I have a question. For example, the log2(0) function has different expressions in the results of spark and mysql. Do we need to adapt them separately?
   Regarding negative tests, I would like to ask which ones are negative tests. At first I thought it was just log2(0).
   Are things like log(2/3) also considered negative tests?
   



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468436756


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,33 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");

Review Comment:
   Yes, but there is also the problem of node type conversion, otherwise it seems to be taking integer bits, which is unreasonable



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468440668


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));

Review Comment:
   I will fix it in next PR



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468735191


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,29 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG2, VmName.EXPAND);
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL);
+    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)", "DOUBLE NOT NULL",
+        "NaN");
+    f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",

Review Comment:
   https://issues.apache.org/jira/browse/CALCITE-6232, Julian seems to think this is not a bug



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai closed pull request #3648: [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library)
URL: https://github.com/apache/calcite/pull/3648


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1925771662

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648) **Quality Gate passed**  
   The SonarCloud Quality Gate passed, but some issues were introduced.
   
   [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_coverage&view=list)  
   [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648)
   
   


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474830504


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,65 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",

Review Comment:
   What happens if you use the decimal form like `.5` instead of `1/2`?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479946888


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   @mihaibudiu @tanclary Or should we deal with this situation?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1916583880

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648) **Quality Gate passed**  
   The SonarCloud Quality Gate passed, but some issues were introduced.
   
   [14 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_coverage&view=list)  
   [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648)
   
   


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1471659722


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,65 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",

Review Comment:
   What happens when you wrap the arguments in casts? 



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,65 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",

Review Comment:
   These are good tests but I don't know that we should merge this until the pretty glaring bug is fixed



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468438469


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));

Review Comment:
   You can see that there are retained decimal places.
   The first one is obviously log2(0) and log10(0). We may need to process this.
   Second, when converting fractions like 2/3 into sqlnode nodes, an integer value is taken, which also needs further modification.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474973925


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,67 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   you have to write tests with illegal and NULL values for all combinations of arguments



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1475796619


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,67 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   done.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477424593


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   I am still of the belief that even though LOG(0) returning `-Infinity` makes sense mathematically, we should do our best to parity the dialects that support these functions (i.e. return NULL or error, depending on the dialect). I am a fan of extending a function's behavior beyond whatever the dialect supports, but not augmenting it.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477424593


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   I am still of the belief that even though LOG(0) returning `-Infinity` makes sense mathematically, we should do our best to parity the dialects that support these functions (i.e. return NULL or error, depending on the dialect). I am a fan of extending a function's behavior, but not augmenting it.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479917763


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Hello, thank you very much for your suggestion. Regarding negative test, I think of the following case
   
   - log10(-1): When the parameter in log10 is a negative number
   - log10(null): When the parameters in log10 are empty
   - log10(-`Infinity`): When the parameter in log10 is -`Infinity`
   - log10(0): When the parameter in log10 is 0
   
   I tested based on mysql. Only when log(0) and log10(-1), calcite cannot match the results in mysql. In mysql, both of them return NULL. When log10(0), calcite returns -`Infinity. `log10(-1) returns NaN
   
   ![2024-02-06 22-20-16屏幕截图](https://github.com/apache/calcite/assets/77189278/27b21670-5364-463c-b6dc-d750c46d7cd1)
   
   
   



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479946888


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   @mihaibudiu Or should we deal with this situation? log10(-1) and log10(0)



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477862898


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   - I have a question. For example, the log2(0) function has different expressions in the results of spark and mysql. Do we need to adapt them separately?
   - Regarding negative tests, I would like to ask which ones are negative tests. At first I thought it was just log2(0).
   - Are things like log(2/3) also considered negative tests?
   



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479946888


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   @mihaibudiu Or should we deal with this situation?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1467680221


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6162,6 +6162,18 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log10(cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f = Fixtures.forOperators(true);
+    f.setFor(SqlStdOperatorTable.LOG2, VmName.EXPAND);
+    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",

Review Comment:
   done, thank you



##########
site/_docs/reference.md:
##########
@@ -1335,6 +1335,7 @@ comp:
 | MOD(numeric1, numeric2)   | Returns the remainder (modulus) of *numeric1* divided by *numeric2*. The result is negative only if *numeric1* is negative
 | SQRT(numeric)             | Returns the square root of *numeric*
 | LN(numeric)               | Returns the natural logarithm (base *e*) of *numeric*
+| LOG2(numeric)             | Returns the base 2 logarithm of *numeric*

Review Comment:
   done, thank you



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1480054578


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   https://github.com/apache/calcite/pull/3669, It seems that there should be the same situation



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479946888


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   @mihaibudiu Or should we deal with this situation?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1475796414


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,65 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",

Review Comment:
   The result looks correct, it's only different when it comes to fractions



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,67 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   done. The result looks correct, it's only different when it comes to fractions



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1467953149


##########
site/_docs/reference.md:
##########
@@ -2775,6 +2775,7 @@ In the following:
 | b f | LENGTH(string)                               | Equivalent to `CHAR_LENGTH(string)`
 | h s | LEVENSHTEIN(string1, string2)                | Returns the Levenshtein distance between *string1* and *string2*
 | b | LOG(numeric1 [, numeric2 ])                    | Returns the logarithm of *numeric1* to base *numeric2*, or base e if *numeric2* is not present
+| m s | | LOG2(numeric)                              | Returns the base 2 logarithm of *numeric*

Review Comment:
   Remove the extra ` |`



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,29 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG2, VmName.EXPAND);
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL);

Review Comment:
   Even if the behavior does not vary across libraries, it is easy to test against all libraries using the `consumer` pattern. See any test that uses `forEachLibrary()` for examples.



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,29 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG2, VmName.EXPAND);
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL);
+    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)", "DOUBLE NOT NULL",
+        "NaN");
+    f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",

Review Comment:
   Using desmos, this should equal ~-0.5849625007



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,29 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG2, VmName.EXPAND);
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL);
+    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)", "DOUBLE NOT NULL",
+        "NaN");
+    f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",

Review Comment:
   wait what, why is log2(2/3) `-Infinity`?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468438469


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));

Review Comment:
   @XuQianJin-Stars @tanclary 
   Hello. You can see that there are retained decimal places.
   1.  We need to perform special processing for log2(0) and log(10)
   2.  When converting fractions like 2/3 into sqlnode nodes, an integer value is taken, which also needs further modification.
   
   



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1913074183

   https://issues.apache.org/jira/browse/CALCITE-6232


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468428068


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -2142,6 +2142,18 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
           OperandTypes.NUMERIC_OPTIONAL_NUMERIC,
           SqlFunctionCategory.NUMERIC);
 
+  /** The "LOG2(value)" function.
+   *
+   * @see SqlStdOperatorTable#LN
+   * @see SqlStdOperatorTable#LOG10
+   */
+  @LibraryOperator(libraries = {MYSQL, SPARK})

Review Comment:
   I noticed that PostgreSQL, SQL Server, and Oracle all support the processing of the LOG2 function. You can check if this link can help improve this PR: https://www.w3resource.com/oracle/oracle-numeric-functions/oracle-log-function.php



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468438469


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));

Review Comment:
   @XuQianJin-Stars @tanclary You can see that there are retained decimal places.
   The first one is obviously log2(0) and log10(0). We may need to process this.
   Second, when converting fractions like 2/3 into sqlnode nodes, an integer value is taken, which also needs further modification.



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));

Review Comment:
   @XuQianJin-Stars @tanclary 
   You can see that there are retained decimal places.
   The first one is obviously log2(0) and log10(0). We may need to process this.
   Second, when converting fractions like 2/3 into sqlnode nodes, an integer value is taken, which also needs further modification.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1921316637

   @mihaibudiu Maybe I need your opinion if you have time, thank you
   


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474711660


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,65 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",

Review Comment:
   > Why don't we fix the bug? Do you have an updated JIRA case for it?
   
   I don't know if this is true. https://issues.apache.org/jira/browse/CALCITE-6232



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477247017


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   @mihaibudiu @tanclary Hello, I seem to have forgotten the simplest truth, that is, there are no fractions in computers, only float/double point numbers. I changed 2/3 in the test to 2.0/3, and it turns out that the result is correct. If 2/3 must output the correct result, then this bug does not only exist in the log function, but the pow and abs functions also have similar problems. Regarding log(0) I don't think it's a bug.
   
   Thank you very much for your guidance on these two PRs of my adaptation function. I am sorry for causing you a lot of trouble. If there is anything missing in this PR, please point it out, thank you.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477247017


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Hello, I seem to have forgotten the simplest truth, that is, there are no fractions in computers, only float/double point numbers. I changed 2/3 in the test to 2.0/3, and it turns out that the result is correct. If 2/3 must output the correct result, then this bug does not only exist in the log function, but the pow and abs functions also have similar problems. Regarding log(0) I don't think it's a bug.
   
   Thank you very much for your guidance on these two PRs of my adaptation function. I am sorry for causing you a lot of trouble. If there is anything missing in this PR, please point it out, thank you.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468438362


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",

Review Comment:
   You can see that there are retained decimal places. 
   The first one is obviously log2(0) and log10(0). We may need to process this. 
   Second, when converting fractions like 2/3 into sqlnode nodes, an integer value is taken, which also needs further modification.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "XuQianJin-Stars (via GitHub)" <gi...@apache.org>.
XuQianJin-Stars commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468431002


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,33 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");

Review Comment:
   `Double.NEGATIVE_INFINITY` is used to represent negative infinity. Precision preservation needs to be given here. For example: mysql is 10 digits.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai closed pull request #3648: [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library)
URL: https://github.com/apache/calcite/pull/3648


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468283719


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,29 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG2, VmName.EXPAND);
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL);
+    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)", "DOUBLE NOT NULL",
+        "NaN");
+    f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",

Review Comment:
   I put the test into log10 and had the same problem



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479917763


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Hello, thank you very much for your suggestion. Regarding negative test, I think of the following case
   
   - log10(-1): When the parameter in log10 is a negative number
   - 
   - log10(null): When the parameters in log10 are empty
   - 
   - log10(-`Infinity`): When the parameter in log10 is -`Infinity`
   - 
   - log10(0): When the parameter in log10 is 0
   
   I tested based on mysql. Only when log(0) and log10(-1), calcite cannot match the results in mysql. In mysql, both of them return NULL. When log10(0), calcite returns -`Infinity. `log10(-1) returns NaN
   
   ![test2](https://github.com/apache/calcite/assets/77189278/c29a69b6-8c4e-4d94-b7c3-148da99f3243)
   
   



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1931497457

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648) **Quality Gate passed**  
   The SonarCloud Quality Gate passed, but some issues were introduced.
   
   [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_coverage&view=list)  
   [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648)
   
   


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai closed pull request #3648: [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library)
URL: https://github.com/apache/calcite/pull/3648


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477438271


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   If a function is part of a dialect, it should definitely have the same behavior as the dialect requires. This should also be documented. We may be dealing with multiple LOG functions, with slightly different behaviors. That's why it is very important to test all corner cases, including writing negative tests.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477897147


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   I have tested in both spark and mysql. The output of log2(0) in spark and mysql is null. Do I need to process the parameter 0 in sqlfunctions? Or simply adding test is enough.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477862898


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   I have a question. For example, the log2(0) function has different expressions in the results of spark and mysql. Do we need to adapt them separately?
   Regarding negative tests, I would like to ask which ones are negative tests. At first I thought it was just log2(0).



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477310637


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   I will remove this part of the test later



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1923453271

   @mihaibudiu @tanclary https://issues.apache.org/jira/browse/CALCITE-6237


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474685867


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,65 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",

Review Comment:
   Why don't we fix the bug? Do you have an updated JIRA case for it?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479946888


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   @mihaibudiu Or should we deal with this situation? log10(-1) and log10(0)



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1481029377


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Since my pr has a conflict, I will close it first, and then I will raise a pr after solving the problem of log2(0) returning null.
   Thanks



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1479937474


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   My current idea is to add a judgment to the log function in sqlFunctions. If it is less than or equal to 0, it will return 0. But there is a problem, that is, log10 and log functions are adapted to the current calcite database. So what should we do? To solve this problem, we cannot guarantee that the results returned by the database will be the same.
   In other words, I should write Sqlfunctions specifically for log2



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468280299


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,29 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG2, VmName.EXPAND);
+    f0.checkFails("^log2(4)^",
+        "No match found for function signature LOG2\\(<NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL);
+    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)", "DOUBLE NOT NULL",
+        "NaN");
+    f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",

Review Comment:
   It seems that there is a little bug in parsing SqlNode. It seems that log10 also has the same problem. I will open a jira to record the problem.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468438362


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",

Review Comment:
   You can see that there are retained decimal places. 
   The first one is obviously log2(0) and log10(0). We may need to process this. 
   Second, when converting fractions like 2/3 into sqlnode nodes, an integer value is taken, which also needs further modification.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468402024


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,30 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {

Review Comment:
   It will be better if there are JIRA document.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function on SqlStdOperatorTable [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1466752322


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6162,6 +6162,18 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log10(cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f = Fixtures.forOperators(true);
+    f.setFor(SqlStdOperatorTable.LOG2, VmName.EXPAND);
+    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",

Review Comment:
   Can we add more tests:
   with decimal arguments, negative arguments, maybe an expression (2 / 3)



##########
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, LOG10, LOG2

Review Comment:
   Is this a standard function? I see it supported for MySQL but no info on the standard.



##########
site/_docs/reference.md:
##########
@@ -1335,6 +1335,7 @@ comp:
 | MOD(numeric1, numeric2)   | Returns the remainder (modulus) of *numeric1* divided by *numeric2*. The result is negative only if *numeric1* is negative
 | SQRT(numeric)             | Returns the square root of *numeric*
 | LN(numeric)               | Returns the natural logarithm (base *e*) of *numeric*
+| LOG2(numeric)             | Returns the base 2 logarithm of *numeric*

Review Comment:
   Earlier in the PR you prioritize LOG10 over LOG2, can you figure out the correct alphabetical ordering and make it consistent throughout the PR?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468436756


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,33 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");

Review Comment:
   Yes, but there is also the problem of node type conversion, otherwise it seems to be taking integer bits, which is unreasonable



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468440668


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,35 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));

Review Comment:
   I will fix it in next PR



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1472236067


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,65 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",

Review Comment:
   @tanclary I'm trying to fix it, but I'm sure it won't last long. I think we have to be honest and tell the user about this bug. Now maybe our difficulty is how to tell the user about this bug, should I add a TODO tag



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474701648


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,65 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    // The fractional conversion of the Log function is reserved only for integer bits
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+          "-Infinity");
+    f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+        "0.0");
+    f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",

Review Comment:
   ok, this may take a while for me because i am learning this



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1468404232


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6188,6 +6188,30 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
     f.checkNull("log(10, cast(null as real))");
   }
 
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      f.setFor(SqlLibraryOperators.LOG2);

Review Comment:
   What error message for  `log2(0)`?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1467679733


##########
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, LOG10, LOG2

Review Comment:
   done, thank you



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1478594969


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   log(2/3) == log(0). Neither of these is a negative test as those are normal, expected inputs. I found this Wiki article on negative testing to be helpful: https://en.wikipedia.org/wiki/Negative_testing . To find a good candidate for a negative test, imagine ALL possible inputs (of any type), and then subtract all EXPECTED inputs. This leaves you with all UNexpected inputs.
   
   If the dialects return different answers for the same expression, yes we need to account for that, how do Spark and MySQL differ?
   
   If those dialects return `null` for 0 then we should make sure we return `null` for zero.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1480254671


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   The examples you gave above are **not** negative tests (aside from the `null` one). Please read my previous comment. What do you expect of the user? It is a numeric function that is designed to support numeric arguments. Anything outside of that is unexpected.



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3648:
URL: https://github.com/apache/calcite/pull/3648#issuecomment-1930051948

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648) **Quality Gate passed**  
   The SonarCloud Quality Gate passed, but some issues were introduced.
   
   [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3648&resolved=false&inNewCodePeriod=true)  
   [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_coverage&view=list)  
   [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3648&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3648)
   
   


-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]

Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1480048466


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6215,6 +6215,71 @@ 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 LOG@ function (enabled in MYSQL, Spark library)</a>. */
+  @Test void testLog2Func() {
+    final SqlOperatorFixture f0 = fixture();
+    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)", "DOUBLE NOT NULL",
+          "NaN");
+      f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+          "-Infinity");
+      f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+          "1.1375035237499351");
+      f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+          "-1.0");
+      f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+          isWithin(1.5849625007211563, 0.000001));
+      f.checkNull("log2(cast(null as real))");
+    };
+    f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6232">[CALCITE-6232]
+   * Using fractions in LOG function does not return correct results</a>. */
+  @Test void testLogFuncByConvert() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",

Review Comment:
   Double.NaN == null?



-- 
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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org