Julian Hyde, could you take a look at the PR in https://github.com/apache/calcite/pull/385 ? This problem is not reproducible in Calcite, since all columns coming from Druid metrics are considered either DOUBLE or LONG, and if a CAST is on the way, we will not push it to Druid. However, when we create our own tables in Druid from Hive, we might have other data types (FLOAT, DECIMAL) coming from the Druid table. Thanks

I see that FRACTIONAL_TYPES = approximate types (DOUBLE + FLOAT) plus DECIMAL. Should we include DECIMAL? I find that it's a mistake to mix exact types with inexact types.

To be clear, I'm not proposing to redefine FRACTIONAL_TYPES. I'm proposing that we use APPROX_TYPES for recognizing aggregation function types.

Julian Hyde, I had to choose FRACTIONAL types because the problem in Hive was with Decimal, so it was intentional. If we use only approximate types, we will still face this issue for decimal type. Maybe another option is to check the scale of the decimal, and generate one type or another depending on that?

So, if Hive has a column c of type DECIMAL(6, 2) stored inside Druid, and a user executes SUM(c), they are going to get a DOUBLE (i.e. 64 bit floating point) result. Rounding may occur. Is the user OK with that?

You are right, that is fair. Rounding might happen. Overflowing might happen too, though I do not know if there is a deterministic behavior to handle it, e.g., defined by the SQL standard.

When I proposed this fix, I was assuming that this is something that we will document properly on the Hive side, so we can support DECIMAL type (similar to what we do with the handling of NULL semantics by Druid). Are you suggesting that we should not support DECIMAL in Druid?

We could check the precision&scale to control overflow, though user needs to be aware of what is going on in Druid and that rounding might happen.

Ashutosh Chauhan, what do you think? Julian Hyde, could you give your opinion about this too?

I don't think we should support DECIMAL in Druid. Druid doesn't support DECIMAL any more than it supports, say, a geospatial data type.

I agree if column is declared as Decimal in hive, user would expect calculations to be precise and doing it in double would surprise users.
However, I think it is worth considering to add a mode accept.approx.resulset (false by default) in Hive/Calcite to allow this. We can use this mode elsewhere also e.g., to use topN queries which also returns approx resultset.
In my experience world of big data, atleast some users are willing to trade speed for accuracy.

Julian Hyde, Ashutosh Chauhan, I have updated the PR in https://github.com/apache/calcite/pull/385 . Could you take a look? Thanks

Jesus Camacho Rodriguez, Looks good, but please update site/_docs/adapter.md for the new connect string property.

Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/0372d23

Thanks for the feedback Julian Hyde

Re-opening. This change breaks DruidAdapterIT:

• All tests in DruidAdapterIT pass against a269a2b, CALCITE-1623.
• As of 49888a6 there is one failure, DruidAdapterIT.testFilterTimestamp. Nishant Bangarwa mentioned this already. I'll look into it. I suspect that it is a plan improvement due to CALCITE-1601.
• The next change, 0372d23, there are 25 failures. The test takes much longer than usual (13 minutes), and it seems that plans have changed, to do much of the aggregation in Calcite rather than in Druid.

Julian Hyde, I am working on it.

Fixed up in http://git-wip-us.apache.org/repos/asf/calcite/commit/952214a .

Checked that all unit tests and Druid tests pass, except DruidAdapterIT.testFilterTimestamp.

Thanks, that's perfect. I have a subsequent change that will fix DruidAdapterIT.testFilterTimestamp.

Resolved in release 1.12.0 (2017-03-24).