mihaibudiu commented on code in PR #4829:
URL: https://github.com/apache/calcite/pull/4829#discussion_r3715075237
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -942,6 +949,80 @@ private List<SqlNode> generateGroupList(Builder builder,
}
}
+ /** Qualifies a group key when its aggregate input renders as a SQL join
+ * whose columns would otherwise be ambiguous. */
+ private SqlNode maybeQualifyJoinKey(SqlNode field, int key,
+ @Nullable SqlJoin fromJoin, RelNode input) {
+ if (fromJoin == null) {
+ return field;
+ }
+ final @Nullable SqlNode qualified = joinField(input, key, fromJoin);
+ return qualified != null ? qualified : field;
+ }
+
+ /** Resolves a field through row-preserving nodes to the join input that
+ * supplies it. */
+ private static @Nullable SqlNode joinField(RelNode input, int field,
+ SqlJoin fromJoin) {
+ if (input instanceof Project) {
+ final Project project = (Project) input;
+ return joinExpression(project.getInput(),
project.getProjects().get(field),
+ fromJoin);
+ }
+ if (input instanceof Filter) {
+ return joinField(((Filter) input).getInput(), field, fromJoin);
+ }
+ if (!(input instanceof Join)) {
Review Comment:
are these all the cases that need to be considered?
How about Correlate?
Or Set operations?
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -12661,6 +12691,176 @@ public Sql schema(CalciteAssert.SchemaSpec
schemaSpec) {
sql(sql).schema(CalciteAssert.SchemaSpec.JDBC_SCOTT).ok(expected);
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7439";>[CALCITE-7439]
+ * RelToSqlConverter emits ambiguous GROUP BY after LEFT JOIN USING with
+ * semi-join rewrite.</a>. */
+ @Test void
testPostgresqlRoundTripDistinctLeftJoinInSubqueryWithSemiJoinRules() {
+ final String query = "WITH product_keys AS (\n"
+ + " SELECT p.\"product_id\",\n"
+ + " (SELECT MAX(p3.\"product_id\")\n"
+ + " FROM \"foodmart\".\"product\" p3\n"
+ + " WHERE p3.\"product_id\" = p.\"product_id\") AS \"mx\"\n"
+ + " FROM \"foodmart\".\"product\" p\n"
+ + ")\n"
+ + "SELECT DISTINCT \"product_id\"\n"
+ + "FROM product_keys pk\n"
+ + "LEFT JOIN \"foodmart\".\"product\" p2 USING (\"product_id\")\n"
+ + "WHERE pk.\"product_id\" IN (\n"
+ + " SELECT p4.\"product_id\"\n"
+ + " FROM \"foodmart\".\"product\" p4\n"
+ + ")";
+
+ final RuleSet rules = semiJoinRules();
+
+ final String generated = sql(query).withPostgresql().optimize(rules,
null).exec();
+ assertThat(generated, containsString("GROUP BY \"t2\".\"product_id\""));
Review Comment:
These tests for substrings do not look particularly robust.
Do you find them valuable? Maybe you can add a comment about what they are
checking.
Isn't the round-trip test enough?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
