[
https://issues.apache.org/jira/browse/CALCITE-4349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17257496#comment-17257496
]
Julian Hyde edited comment on CALCITE-4349 at 1/2/21, 7:49 PM:
---------------------------------------------------------------
Looks basically good. But it touches more files/classes than I would like,
considering that it is just an alternative syntax for something we already do.
* I don't think a new {{SqlSyntax}} enum is needed. Could you extend
ORDERED_FUNCTION to allow SEPARATOR?
* Does {{SEPARATOR}} keyword really need to be reserved?
* I presume changes to {{dummy.iq}} are accidental. Remove them.
* Are there tests that GROUP_CONCAT only works if {{fun=mysql}}?
* [MySQL
reference|https://dev.mysql.com/doc/refman/8.0/en/aggregate-functions.html#function_group-concat]
implies that {{GROUP_CONCAT}} accepts composite arguments, e.g.
{{GROUP_CONCAT(x, y SEPARATOR ';')}}. I don't know whether it really accepts
composite arguments. Please find out, and add a positive or negative test.
* You don't need {{class SqlSeparatorOperator}}. See how
{{SqlStdOperatorTable.DESC}} does it.
* Javadoc for GROUP_CONCAT mentions STRING_AGG. Copy-paste error?
* Method {{isAllowsSeparator}} should be named {{allowsSeparator}}, must be
defined in SqlAggFunction, needs javadoc. I'm a bit surprised that it is never
called.
* {{allowsSeparatorTreatment}} is a copy-paste error. Please be more careful.
was (Author: julianhyde):
Looks basically good. But it touches more files/classes than I would like,
considering that it is just an alternative syntax for something we already do.
* I don't think a new {{SqlSyntax}} enum is needed. Could you extend
ORDERED_FUNCTION to allow SEPARATOR?
* Does {{SEPARATOR}} keyword really need to be reserved?
* I presume changes to {{dummy.iq}} are accidental. Remove them.
* Are there tests that GROUP_CONCAT only works if {{fun=mysql}}?
* [MySQL
reference](https://dev.mysql.com/doc/refman/8.0/en/aggregate-functions.html#function_group-concat)
implies that {{GROUP_CONCAT}} accepts composite arguments, e.g.
{{GROUP_CONCAT(x, y SEPARATOR ';')}}. I don't know whether it really accepts
composite arguments. Please find out, and add a positive or negative test.
* You don't need {{class SqlSeparatorOperator}}. See how
{{SqlStdOperatorTable.DESC}} does it.
* Javadoc for GROUP_CONCAT mentions STRING_AGG. Copy-paste error?
* Method {{isAllowsSeparator}} should be named {{allowsSeparator}}, must be
defined in SqlAggFunction, needs javadoc. I'm a bit surprised that it is never
called.
* {{allowsSeparatorTreatment}} is a copy-paste error. Please be more careful.
> Support GROUP_CONCAT aggregate function for MySQL
> -------------------------------------------------
>
> Key: CALCITE-4349
> URL: https://issues.apache.org/jira/browse/CALCITE-4349
> Project: Calcite
> Issue Type: Bug
> Reporter: Julian Hyde
> Assignee: Zhen Wang
> Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Support the {{GROUP_CONCAT}} aggregate function for MySQL. Here is the
> [syntax|https://dev.mysql.com/doc/refman/8.0/en/aggregate-functions.html#function_group-concat]:
> {noformat}
> GROUP_CONCAT([DISTINCT] expr [,expr ...]
> [ORDER BY {unsigned_integer | col_name | expr}
> [ASC | DESC] [,col_name ...]]
> [SEPARATOR str_val])
> {noformat}
>
> {{GROUP_CONCAT}} is analogous to {{LISTAGG}} (see CALCITE-2754) (and also to
> BigQuery and PostgreSQL's {{STRING_AGG}}, see CALCITE-4335). For example, the
> query
> {code:java}
> SELECT deptno, GROUP_CONCAT(ename ORDER BY empno SEPARATOR ';')
> FROM Emp
> GROUP BY deptno
> {code}
> is equivalent to (and in Calcite's algebra would be desugared to)
> {code:java}
> SELECT deptno, LISTAGG(ename, ';') WITHIN GROUP (ORDER BY empno)
> FROM Emp
> GROUP BY deptno
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
