Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2686
Merging
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2686
Great! +1 to merge :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user wuchong commented on the issue:
https://github.com/apache/flink/pull/2686
@tonycox Yes, the second option support BigDecimal only in SQL. I think
we could put the new `power` function in `BuiltInMethods`. Or create a util
class in
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2686
I agree with @wuchong that we need create a new issue in JIRA for
supporting `BigDecimal` in functions (`log`,`ln`,`exp` etc), but I suppose that
if problem in calcite with `DECIMAL` is done we will
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/2686
I got a bit lost in the discussion, but if I understood @wuchong's options
right, I prefer the first option. Automatically converting `DECIMAL` to
`DOUBLE` can result in rounding errors. However,
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2686
@wuchong if choosing second option, why not to solve this issue
https://issues.apache.org/jira/browse/CALCITE-1468 instead of solve on flink
side ?
---
If your project is set up for it, you can
Github user wuchong commented on the issue:
https://github.com/apache/flink/pull/2686
Hi @tonycox , I agree with what Fabian said, it's the same as I mean in
this
[comment](https://github.com/apache/flink/pull/2686#issuecomment-256963469).
What I'm talking about POWER_DEC
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2686
Hi @wuchong , native java implementation of "power" function suits to most
cases. So i deleted POWER_DEC from flink, because it has no proper types.
Actually we don't make user's life harder.
Github user wuchong commented on the issue:
https://github.com/apache/flink/pull/2686
Hi @tonycox , I think the POWER_DEC could be fixed in Flink side. That's
why I said we need to implement a `power(double, BigDecimal)` method in Flink.
And bind BuiltInMethods.POWER_DEC to the new
Github user tonycox commented on the issue:
https://github.com/apache/flink/pull/2686
Hi @wuchong
I agree that users can cast types manually. And that
BuiltInMethods.POWER_DEC should be bound power(double, BigDecimal), but it's
trouble of Calcite
Github user wuchong commented on the issue:
https://github.com/apache/flink/pull/2686
According to the question mentioned in the dev mailing. I prefer to
approach 2). POWER only works on double. And `power(double, BigDecimal)` is
only used to support `SQRT(double)` which is actually
Github user wuchong commented on the issue:
https://github.com/apache/flink/pull/2686
Hi @fhueske @tonycox , the PR is really good.
But I think the true reason of this issue is because the
`BuiltInMethods.POWER_DEC` is bound to a wrong method, which should be
`power(double,
12 matches
Mail list logo
