You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/01/05 16:06:46 UTC
[GitHub] [calcite] XuQianJin-Stars opened a new pull request #1723:
[CALCITE-3684] Implement CBRT function
XuQianJin-Stars opened a new pull request #1723: [CALCITE-3684] Implement CBRT function
URL: https://github.com/apache/calcite/pull/1723
As illustrated in [CALCITE-3684](https://issues.apache.org/jira/browse/CALCITE-3684)
cbrt(x) → double Returns the cube root of x.
sql: `select cbrt(-8);`
result:
| EXPR$0 |
| ---- |
| -2.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] amaliujia commented on issue #1723: [CALCITE-3684]
Implement CBRT function
Posted by GitBox <gi...@apache.org>.
amaliujia commented on issue #1723: [CALCITE-3684] Implement CBRT function
URL: https://github.com/apache/calcite/pull/1723#issuecomment-570963221
I have an impression that now we encourage to add tests into `functions.iq`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] chunweilei closed pull request #1723: [CALCITE-3684]
Implement CBRT function
Posted by GitBox <gi...@apache.org>.
chunweilei closed pull request #1723: [CALCITE-3684] Implement CBRT function
URL: https://github.com/apache/calcite/pull/1723
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] chunweilei commented on a change in pull request #1723:
[CALCITE-3684] Implement CBRT function
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #1723: [CALCITE-3684] Implement CBRT function
URL: https://github.com/apache/calcite/pull/1723#discussion_r363148268
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -1384,6 +1384,17 @@ public static double atan2(double b0, double b1) {
return Math.atan2(b0, b1);
}
+ // CBRT
+ /** SQL <code>CBRT</code> operator applied to BigDecimal values. */
+ public static double cbrt(BigDecimal b0) {
+ return Math.cbrt(b0.doubleValue());
+ }
+
+ /** SQL <code>CBRT</code> operator applied to double values. */
+ public static double cbrt(double b0) {
+ return Math.cbrt(b0);
+ }
Review comment:
Maybe we don't need to name the parameter as `b0` since there is only one parameter. I think `b` is fine.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] chunweilei commented on a change in pull request #1723:
[CALCITE-3684] Implement CBRT function
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #1723: [CALCITE-3684] Implement CBRT function
URL: https://github.com/apache/calcite/pull/1723#discussion_r363148010
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -1384,6 +1384,17 @@ public static double atan2(double b0, double b1) {
return Math.atan2(b0, b1);
}
+ // CBRT
+ /** SQL <code>CBRT</code> operator applied to BigDecimal values. */
+ public static double cbrt(BigDecimal b0) {
+ return Math.cbrt(b0.doubleValue());
+ }
Review comment:
How about changing `return Math.cbrt(b0.doubleValue());` to `cbrt(b0.doubleValue()`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] XuQianJin-Stars commented on a change in pull request
#1723: [CALCITE-3684] Implement CBRT function
Posted by GitBox <gi...@apache.org>.
XuQianJin-Stars commented on a change in pull request #1723: [CALCITE-3684] Implement CBRT function
URL: https://github.com/apache/calcite/pull/1723#discussion_r363157427
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -1384,6 +1384,17 @@ public static double atan2(double b0, double b1) {
return Math.atan2(b0, b1);
}
+ // CBRT
+ /** SQL <code>CBRT</code> operator applied to BigDecimal values. */
+ public static double cbrt(BigDecimal b0) {
+ return Math.cbrt(b0.doubleValue());
+ }
+
+ /** SQL <code>CBRT</code> operator applied to double values. */
+ public static double cbrt(double b0) {
+ return Math.cbrt(b0);
+ }
Review comment:
> Maybe we don't need to name the parameter as `b0` since there is only one parameter. I think `b` is fine.
well, I have changed 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] XuQianJin-Stars commented on issue #1723: [CALCITE-3684]
Implement CBRT function
Posted by GitBox <gi...@apache.org>.
XuQianJin-Stars commented on issue #1723: [CALCITE-3684] Implement CBRT function
URL: https://github.com/apache/calcite/pull/1723#issuecomment-570970363
> I have an impression that now we encourage to add tests into `functions.iq`
hi @amaliujia Thank you very much for reminding me.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services