[GitHub] [calcite] libenchao commented on pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation

[GitBox <gi...@apache.org>]

libenchao commented on PR #2868:
URL: https://github.com/apache/calcite/pull/2868#issuecomment-1312638552

   > As for PERCENTILE_CONT, based on conversation with Julian in the Jira ticket, there is nothing to be done.
   
   I'm not sure about this conclusion, I've checked the SQL standard (2011), 10.9:
   
   > If PERCENTILE_CONT is specified, then:
   > 1) Let ROW0 be the greatest exact numeric value with scale 0 (zero) that is less than or equal
   > to NVE*(N–1). Let ROWLIT0 be a <literal> representing ROW0.
   > 2) Let ROW1 be the least exact numeric value with scale 0 (zero) that is greater than or equal to NVE*(N–1). Let ROWLIT1 be a <literal> representing ROW1.
   > 3) Let FACTOR be an <approximate numeric literal> representing NVE*(N–1)–ROW0.
   > 4) The result is the result of the <scalar subquery>
   >      ( WITH TEMPTABLE(X, Y) AS
   >            ( SELECT ROW_NUMBER()
   > OVER (ORDER BY WSP) - 1, TXCOLNAME
   > FROM TXANAME )
   > SELECT CAST ( T0.Y + FACTOR * (T1.Y - T0.Y) AS DT ) FROM TEMPTABLE T0, TEMPTABLE T1
   > WHERE T0.ROWNUMBER = ROWLIT0
   > AND T1.ROWNUMBER = ROWLIT1 )
   
   We can see that the standard is using `CAST ( T0.Y + FACTOR * (T1.Y - T0.Y) AS DT )` as the result for `PERCENTILE_COUT`, whose type is the same as the sort key.
   
   And also I've checked some databases, and found: [oracle](https://docs.oracle.com/cd/B19306_01/server.102/b14200/functions110.htm) and [postgrelsql](https://www.postgresql.org/docs/9.4/functions-aggregate.html) are consistent with the standard.
   
   There are some databases, such as [server sql](https://learn.microsoft.com/en-us/sql/t-sql/functions/percentile-cont-transact-sql?view=sql-server-ver16), are declaring the return type which is not consistent with the standard.
   
   Hence, IMO, we should make it consistent with the standard by default. Of course if could be extensible for some dialects, if we want to achieve that, maybe the proper place to add the extension is in `RelDataTypeSystem`


-- 
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