I had tried adding AggregateReduceFunctionsRule to the planner and OptCluster but for some reason couldn't get SqlToRelConverter to expand a simple non-windowed aggregate, e.g. select STDDEV_POP(x) from db;
On Fri, May 5, 2017 at 2:11 PM, Julian Hyde <[email protected]> wrote: > You're basically describing what it would take to implement the CORR > aggregate function. If you do the steps you describe it would work for > everyone. > > I know that regular (non-windowed) aggregates and windowed aggregates > are expanded via different mechanisms. It would be good to unify the > mechanisms one day. But until then, regular aggregates expanded using > AggregateReduceFunctionsRule.reduceAgg, and it seems to work. > > On Fri, May 5, 2017 at 1:26 PM, Dmitri Shtilman <[email protected]> wrote: > > Hi Julian, > > > > Yes, CORR is rightly a reserved keyword, I just think it would be useful > if > > Calcite would also recognize it as an aggregate, even if it's not yet > > implemented. It could then be expanded by the user if necessary. > > > > A little background on our use case. Our project supports some aggregates > > natively (e.g. AVG) while other aggregates, like COVAR_POP and > STDDEV_POP, > > are expanded manually. We wanted to expand `CORR` as well: CORR(x, y) > > ==> COVAR_POP(x, > > y) / (STDDEV_POP(x) * STDDEV_POP(y)), but `CORR` is being flagged down by > > the parser as unexpected in the context. > > > > By the way, regarding the expansion, we tried to trigger this selective > > expansion through StandardConvertletTabl > > <https://github.com/apache/calcite/pull/16/commits/ > 009c60b83551a515b5d77d6dbefd838ae2a0a820#diff- > dec914cbd5702dabbf1f48b5e208a46d> > > e, AvgVarianceConvertlet and AggregateReduceFunctionsRule opt rule, > however > > found that the expansion is only enabled for windowed aggregates > (starting > > with [CALCITE-750] commit 918e612bbb4da90ad94b283c1f06cf055102c09e), so > > switched to manual expansion. If you think there's a better way to handle > > this, please share your thoughts. > > > > Here is the JIRA issue to implement missing aggregates: > > https://issues.apache.org/jira/browse/CALCITE-1776 > > > > Thanks, > > Dmitri > > > > > > On Thu, May 4, 2017 at 11:50 AM, Julian Hyde <[email protected]> wrote: > > > >> “CORR” is a reserved keyword in the parser because it is a reserved > >> keyword in the SQL:2011 and SQL:2014 draft standards. Even though the > >> aggregate function is not implemented, it’s better that the parser > gives an > >> error, so people find out early that “CORR” is a bad name for a table or > >> column. > >> > >> Can you please log a JIRA case to implement the CORR aggregate function? > >> The work is probably similar to what was done[1] in > >> https://issues.apache.org/jira/browse/CALCITE-422 < > >> https://issues.apache.org/jira/browse/CALCITE-422> to implement > REGR_SXX > >> and REGR_SYY. > >> > >> I notice that REGR_SLOPE, REGR_INTERCEPT, REGR_COUNT, REGR_R2, > REGR_AVGX, > >> REGR_AVGY, REGR_SXY are not implemented either. > >> > >> Julian > >> > >> [1] https://git1-us-west.apache.org/repos/asf?p=calcite.git;a= > >> commit;h=af86cd87 <https://git1-us-west.apache. > >> org/repos/asf?p=calcite.git;a=commit;h=af86cd87> > >> > >> > >> > >> > On May 3, 2017, at 4:45 PM, Dmitri Shtilman <[email protected]> wrote: > >> > > >> > Hi all, > >> > > >> > A quick question regarding CORR(x,y). > >> > It appears that Calcite recognizes "CORR" a reserved keyword but the > >> parser > >> > doesn't recognize it as an aggregate, like for example COVAR_POP. > Could > >> > CORR be added to the list of recognized aggregates? > >> > > >> > Thanks, > >> > Dmitri > >> > > >> > > >> > ERROR-- Parse failed: line 1, column 8, Encountered "CORR" at line 1, > >> column 8. > >> > Was expecting one of: > >> > "UNION" ... > >> > "INTERSECT" ... > >> > ... > >> > "COVAR_POP" ... > >> > "COVAR_SAMP" ... > >> > ... > >> > >> >
