You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2022/05/20 08:43:57 UTC
[GitHub] [calcite] wojustme opened a new pull request, #2812: [CALCITE-5061] MysqlSqlDialect support to unparse LISTAGG aggregate function
wojustme opened a new pull request, #2812:
URL: https://github.com/apache/calcite/pull/2812
ISSUE Link: https://issues.apache.org/jira/browse/CALCITE-5163
Simple Description:
MysqlSqlDialect translate LISTAGG to GROUP_CONCAT, when meeting `SqlKind#WITHIN_GROUP` or `SqlKind#LISTAGG`.
--
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
[GitHub] [calcite] chunweilei merged pull request #2812: [CALCITE-5163] MysqlSqlDialect support to unparse LISTAGG aggregate function
Posted by GitBox <gi...@apache.org>.
chunweilei merged PR #2812:
URL: https://github.com/apache/calcite/pull/2812
--
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
[GitHub] [calcite] wojustme commented on pull request #2812: [CALCITE-5163] MysqlSqlDialect support to unparse LISTAGG aggregate function
Posted by GitBox <gi...@apache.org>.
wojustme commented on PR #2812:
URL: https://github.com/apache/calcite/pull/2812#issuecomment-1135434825
Hi @chunweilei
Thanks for your reviewing. And code has been updated, please take your time to review it again.
Thanks a lot.
--
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
[GitHub] [calcite] wojustme commented on a diff in pull request #2812: [CALCITE-5163] MysqlSqlDialect support to unparse LISTAGG aggregate function
Posted by GitBox <gi...@apache.org>.
wojustme commented on code in PR #2812:
URL: https://github.com/apache/calcite/pull/2812#discussion_r880076497
##########
core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java:
##########
@@ -223,11 +229,56 @@ public MysqlSqlDialect(Context context) {
unparseFloor(writer, call);
break;
+ case WITHIN_GROUP:
+ final List<SqlNode> operands = call.getOperandList();
+ if (operands.size() <= 0 || operands.get(0).getKind() != SqlKind.LISTAGG) {
+ super.unparseCall(writer, call, leftPrec, rightPrec);
+ return;
+ }
+ unparseListAggCall(writer, (SqlCall) operands.get(0),
+ operands.size() == 2 ? operands.get(1) : null, leftPrec, rightPrec);
+ break;
+
+ case LISTAGG:
+ unparseListAggCall(writer, call, null, leftPrec, rightPrec);
+ break;
+
default:
super.unparseCall(writer, call, leftPrec, rightPrec);
}
}
+ /**
+ * Unparses LISTAGG for MySQL. This call is translated to GROUP_CONCAT.
+ *
+ * @param writer Writer
+ * @param listAggCall Call of LISTAGG
+ * @param orderItemNode Elems of WITHIN_GROUP, NULL means none elem in the WITHIN_GROUP
+ * @param leftPrec Origin leftPrec
+ * @param rightPrec Origin rightPrec
+ */
+ private void unparseListAggCall(SqlWriter writer, SqlCall listAggCall,
+ @Nullable SqlNode orderItemNode, int leftPrec, int rightPrec) {
+ // LISTAGG(DISTINCT c1, ',') WITHIN GROUP (ORDER BY c2, c3)
+ // GROUP_CONCAT(DISTINCT c1 ORDER BY c2, c3 DESC SEPARATOR ',')
+ final List<SqlNode> listAggCallOperands = listAggCall.getOperandList();
Review Comment:
OK.
--
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
[GitHub] [calcite] chunweilei commented on a diff in pull request #2812: [CALCITE-5163] MysqlSqlDialect support to unparse LISTAGG aggregate function
Posted by GitBox <gi...@apache.org>.
chunweilei commented on code in PR #2812:
URL: https://github.com/apache/calcite/pull/2812#discussion_r879990874
##########
core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java:
##########
@@ -223,11 +229,56 @@ public MysqlSqlDialect(Context context) {
unparseFloor(writer, call);
break;
+ case WITHIN_GROUP:
+ final List<SqlNode> operands = call.getOperandList();
+ if (operands.size() <= 0 || operands.get(0).getKind() != SqlKind.LISTAGG) {
+ super.unparseCall(writer, call, leftPrec, rightPrec);
+ return;
+ }
+ unparseListAggCall(writer, (SqlCall) operands.get(0),
+ operands.size() == 2 ? operands.get(1) : null, leftPrec, rightPrec);
+ break;
+
+ case LISTAGG:
+ unparseListAggCall(writer, call, null, leftPrec, rightPrec);
+ break;
+
default:
super.unparseCall(writer, call, leftPrec, rightPrec);
}
}
+ /**
+ * Unparses LISTAGG for MySQL. This call is translated to GROUP_CONCAT.
+ *
+ * @param writer Writer
+ * @param listAggCall Call of LISTAGG
+ * @param orderItemNode Elems of WITHIN_GROUP, NULL means none elem in the WITHIN_GROUP
+ * @param leftPrec Origin leftPrec
+ * @param rightPrec Origin rightPrec
+ */
+ private void unparseListAggCall(SqlWriter writer, SqlCall listAggCall,
+ @Nullable SqlNode orderItemNode, int leftPrec, int rightPrec) {
+ // LISTAGG(DISTINCT c1, ',') WITHIN GROUP (ORDER BY c2, c3)
+ // GROUP_CONCAT(DISTINCT c1 ORDER BY c2, c3 DESC SEPARATOR ',')
+ final List<SqlNode> listAggCallOperands = listAggCall.getOperandList();
Review Comment:
Could you please move comments to the java doc comment?
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -2457,6 +2457,30 @@ private SqlDialect nonOrdinalDialect() {
sql(query).withMysql().ok(expected);
}
+ @Test void testMySqlGroupConcat() {
+ final String query = "select\n"
+ + "listagg(distinct \"product_name\", ',') within group(order by \"cases_per_pallet\"),\n"
+ + "listagg(\"product_name\", ',') within group(order by \"cases_per_pallet\"),\n"
Review Comment:
From my perspective, it would be better to change the test name to `testMySqlUnparseListAggCall`.
##########
core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java:
##########
@@ -223,11 +229,56 @@ public MysqlSqlDialect(Context context) {
unparseFloor(writer, call);
break;
+ case WITHIN_GROUP:
+ final List<SqlNode> operands = call.getOperandList();
+ if (operands.size() <= 0 || operands.get(0).getKind() != SqlKind.LISTAGG) {
+ super.unparseCall(writer, call, leftPrec, rightPrec);
+ return;
+ }
+ unparseListAggCall(writer, (SqlCall) operands.get(0),
+ operands.size() == 2 ? operands.get(1) : null, leftPrec, rightPrec);
+ break;
+
+ case LISTAGG:
+ unparseListAggCall(writer, call, null, leftPrec, rightPrec);
+ break;
+
default:
super.unparseCall(writer, call, leftPrec, rightPrec);
}
}
+ /**
+ * Unparses LISTAGG for MySQL. This call is translated to GROUP_CONCAT.
+ *
+ * @param writer Writer
+ * @param listAggCall Call of LISTAGG
+ * @param orderItemNode Elems of WITHIN_GROUP, NULL means none elem in the WITHIN_GROUP
+ * @param leftPrec Origin leftPrec
+ * @param rightPrec Origin rightPrec
+ */
+ private void unparseListAggCall(SqlWriter writer, SqlCall listAggCall,
+ @Nullable SqlNode orderItemNode, int leftPrec, int rightPrec) {
+ // LISTAGG(DISTINCT c1, ',') WITHIN GROUP (ORDER BY c2, c3)
+ // GROUP_CONCAT(DISTINCT c1 ORDER BY c2, c3 DESC SEPARATOR ',')
+ final List<SqlNode> listAggCallOperands = listAggCall.getOperandList();
Review Comment:
Could you please move comments to the java doc comment?
--
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
[GitHub] [calcite] wojustme commented on a diff in pull request #2812: [CALCITE-5163] MysqlSqlDialect support to unparse LISTAGG aggregate function
Posted by GitBox <gi...@apache.org>.
wojustme commented on code in PR #2812:
URL: https://github.com/apache/calcite/pull/2812#discussion_r880077053
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -2457,6 +2457,30 @@ private SqlDialect nonOrdinalDialect() {
sql(query).withMysql().ok(expected);
}
+ @Test void testMySqlGroupConcat() {
+ final String query = "select\n"
+ + "listagg(distinct \"product_name\", ',') within group(order by \"cases_per_pallet\"),\n"
+ + "listagg(\"product_name\", ',') within group(order by \"cases_per_pallet\"),\n"
Review Comment:
Make sense.
--
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
[GitHub] [calcite] wojustme commented on pull request #2812: [CALCITE-5163] MysqlSqlDialect support to unparse LISTAGG aggregate function
Posted by GitBox <gi...@apache.org>.
wojustme commented on PR #2812:
URL: https://github.com/apache/calcite/pull/2812#issuecomment-1136786888
@chunweilei Thanks for your reviewing.
I have tidied up some pr's commit, and force-push it.
--
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