You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "LantaoJin (via GitHub)" <gi...@apache.org> on 2025/02/13 08:50:52 UTC
[PR] [CALCITE-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
LantaoJin opened a new pull request, #4193:
URL: https://github.com/apache/calcite/pull/4193
To reproduce (agg.iq)
```
!use post
!set outputformat mysql
SELECT avg(deptno) as a FROM emp;
```
Should return 22.1429, but returns 22.
The current default logic of Calcite setting the return type to be same as the input column. For example, if the input column is INT, the return type for AVG() is INT.
This behavior is different from other databases. For example:
Postgres
> numeric for any integer type argument, double precision for a floating-point argument, otherwise the same as the argument data type
MySQL
> The SUM() and AVG() functions return a DECIMAL value for exact-value arguments (integer or DECIMAL), and a DOUBLE value for approximate-value arguments (FLOAT or DOUBLE).
Presto
> avg(x) → double. (https://prestodb.io/docs/current/functions/aggregate.html)
Besides, since the `AVG` agg with integer argument returns integer type, the behaviors of `var_pop`, `var_samp`, `stddev_pop`, `stddev_samp` and `stddev` are same as `avg`.
This PR targets to change the default return type of `avg` to double precision when its argument type are `TINYINT`, `SMALLINT`, `INTEGER` or `BIGINT`.
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4193:
URL: https://github.com/apache/calcite/pull/4193#issuecomment-4437394245
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4193: [CALCITE-6830] AVG() returns double precision by default when its argument type is INT_TYPES
URL: https://github.com/apache/calcite/pull/4193
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4193:
URL: https://github.com/apache/calcite/pull/4193#issuecomment-3445640841
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4193:
URL: https://github.com/apache/calcite/pull/4193#issuecomment-2658828752
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4193) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4193&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4193&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4193&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4193&metric=new_coverage&view=list)
 [23.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4193&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4193)
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "LantaoJin (via GitHub)" <gi...@apache.org>.
LantaoJin commented on PR #4193:
URL: https://github.com/apache/calcite/pull/4193#issuecomment-2658723825
> DOUBLE is almost never the right type in SQL, since computations on double values are in general non-deterministic. If any type is right, a variant of DECIMAL should be used for AVG. Can you propose an algorithm to choose the precision and scale for the result?
The `CUME_DIST` and `PERCENT_RANK` return DOUBLE type in Calcite too. Any specific reason to use DECIMAL for AVG? And in Calcite, the default precision is 15 for DOUBLE, 17 for DECIMAL. Will using the default DOUBLE precision in `AVG` result in non-deterministic? In Postgres, the DOUBLE precision for `AVG` calculated based on the input values as follows (not quite sure for now):
> precision = max(precision(input_values) + ceil(log10(count(input_values))), scale(input_values) + decimal_places)
Not sure what precision algorithm would be the best, but definitely not return integer for `AVG` by default.
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4193:
URL: https://github.com/apache/calcite/pull/4193#issuecomment-2746802860
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4193:
URL: https://github.com/apache/calcite/pull/4193#discussion_r1964884456
##########
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystemImpl.java:
##########
@@ -363,7 +363,12 @@ && getDefaultPrecision(typeName) != RelDataType.PRECISION_NOT_SPECIFIED) {
@Override public RelDataType deriveAvgAggType(RelDataTypeFactory typeFactory,
RelDataType argumentType) {
- return argumentType;
+ if (SqlTypeName.INT_TYPES.contains(argumentType.getSqlTypeName())) {
Review Comment:
the type system is actually pluggable. See https://github.com/feldera/feldera/issues/3588 for the right way to do this. We should not change RelDataTypeSystemImpl.
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4193:
URL: https://github.com/apache/calcite/pull/4193#issuecomment-2657255274
DOUBLE is almost never the right type in SQL, since computations on double values are in general non-deterministic. If any type is right, a variant of DECIMAL should be used for AVG. Can you propose an algorithm to choose the precision and scale for the result? Ideally Calcite would follow what other databases do - at least databases that have proper DECIMAL types; the Postgres DECIMAL type is not a standard one, for example.
For functions like STDDEV it's a bit more complicated, since they involve a square root at the last step.
Moreover, this change is backwards-incompatible, and it may cause problems for all projects that have relied on this behavior. At the very least this change should be documented in history.md as such.
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4193:
URL: https://github.com/apache/calcite/pull/4193#issuecomment-2660027904
In general, the result of double addition aggregation depends on the order in which the numbers are processed, whereas the addition of decimals gives always the same result.
It's true that this can only happen for very large integers, since double has 53 bits of mantissa. So any integer value which requires less than 53 bits will be represented exactly, and if all intermediate addition results are less than 53 bits the result will be deterministic and exact; the only imprecision will be produced by the final division. The result of division in FP and DECIMAL can also be slightly different. In general for financial-type computations you should always use DECIMAL.
Your formula depends on the number of input values, which is not known when you compile the SQL program. Calcite is statically-typed, so it has to choose the type at compile-time.
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4193:
URL: https://github.com/apache/calcite/pull/4193#discussion_r1956582411
##########
core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml:
##########
@@ -1619,9 +1619,9 @@ LogicalAggregate(group=[{0}], EXPR$1=[STDDEV_POP($1)], EXPR$2=[AVG($1)], EXPR$3=
</Resource>
<Resource name="planAfter">
<![CDATA[
-LogicalProject(NAME=[$0], EXPR$1=[CAST(POWER(/(-($1, /(*($2, $2), $3)), $3), 0.5:DECIMAL(2, 1))):INTEGER NOT NULL], EXPR$2=[CAST(/($2, $3)):INTEGER NOT NULL], EXPR$3=[CAST(POWER(/(-($1, /(*($2, $2), $3)), CASE(=($3, 1), null:BIGINT, -($3, 1))), 0.5:DECIMAL(2, 1))):INTEGER], EXPR$4=[CAST(/(-($1, /(*($2, $2), $3)), $3)):INTEGER NOT NULL], EXPR$5=[CAST(/(-($1, /(*($2, $2), $3)), CASE(=($3, 1), null:BIGINT, -($3, 1)))):INTEGER NOT NULL])
+LogicalProject(NAME=[$0], EXPR$1=[POWER(/(-($1, /(*(CAST($2):DOUBLE NOT NULL, CAST($2):DOUBLE NOT NULL), $3)), $3), 0.5:DECIMAL(2, 1))], EXPR$2=[/(CAST($2):DOUBLE NOT NULL, $3)], EXPR$3=[POWER(/(-($1, /(*(CAST($2):DOUBLE NOT NULL, CAST($2):DOUBLE NOT NULL), $3)), CASE(=($3, 1), null:BIGINT, -($3, 1))), 0.5:DECIMAL(2, 1))], EXPR$4=[/(-($1, /(*(CAST($2):DOUBLE NOT NULL, CAST($2):DOUBLE NOT NULL), $3)), $3)], EXPR$5=[CAST(/(-($1, /(*(CAST($2):DOUBLE NOT NULL, CAST($2):DOUBLE NOT NULL), $3)), CASE(=($3, 1), null:BIGINT, -($3, 1)))):DOUBLE NOT NULL])
Review Comment:
As you see, the type inferred for the result will cascade through the query plan, influencing the type all values that depend on the aggregation result.
--
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-6830] AVG() returns double precision by default when its argument type is INT_TYPES [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4193:
URL: https://github.com/apache/calcite/pull/4193#issuecomment-3888597262
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
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