-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CALCITE-4924] REGR_SXX and similar aggregate functions return the wr… #3928
base: main
Are you sure you want to change the base?
Conversation
@@ -15,9 +15,17 @@ | |||
# See the License for the specific language governing permissions and | |||
# limitations under the License. | |||
# | |||
!use post | |||
values 1; | |||
!use scott |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is for review.
# [CALCITE-4924] REGR_SXX and similar aggregate functions return the wrong data type | ||
SELECT | ||
MONTH(HIREDATE) as "MONTH", | ||
covar_samp(SAL, COMM) as "COVAR_SAMP(SAL, COMM)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compared to the previous one, this test can remove the cast.
RelDataType arg0Type, RelDataType arg1Type) { | ||
switch (sqlKind) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is a reasonable implementation, but from the test cases, it seems feasible.
Quality Gate passedIssues Measures |
Today all Calcite aggregation functions (except COUNT) return the exact same data type as the data that is being aggregated. This behavior is permitted by the standard, and I think it's a reasonable one. From the Oracle documentation I think that Oracle behaves in the same way. Before I can review this PR we have to agree what you think the "correct" behavior is. Maybe this should also be a property of the type system if you really want to emulate multiple databases. |
This is my idea, and I am glad to receive your feedback on this issue. I will also check if other databases adopt a similar approach. |
Maybe we should discuss the design in the JIRA ticket, I think that's the expected way. |
…ong data type