rubenada commented on code in PR #4115:
URL: https://github.com/apache/calcite/pull/4115#discussion_r1899358078


##########
core/src/test/java/org/apache/calcite/rel/metadata/RelMdUtilTest.java:
##########
@@ -110,4 +117,39 @@ final RelMetadataFixture sql(String sql) {
     });
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6749";>[CALCITE-6749]
+   * RelMdUtil#setAggChildKeys may return an incorrect result</a>. */
+  @Test void testSetAggChildKeys() {

Review Comment:
   Basically we need to convert: "field index X on the Aggregate corresponds to 
which field index Y on the Aggregate's input?"
   If we have this plan
   ```
   LogicalAggregate(group=[{0}], EXPR$1=[COUNT(DISTINCT $1)])
     LogicalProject(DEPTNO=[$9], JOB=[$2])
       LogicalJoin(condition=[=($7, $9)], joinType=[right])
         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
         LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
   ```
   The Aggregate rowType has two fields (0 and 1), the first one (index 0) 
corresponds to the group 0, and the second one (index 1) corresponds to the 
argCall COUNT(DISTINCT $1). In this case, if we want to convert into the 
input's, we'll have 0 => 0, and 1 => 1; there is no problem on the current code.
   
   However, if we have this equivalent plan (same query plan, but without the 
intermediate Project):
   ```
   LogicalAggregate(group=[{9}], EXPR$1=[COUNT(DISTINCT $2)])
     LogicalJoin(condition=[=($7, $9)], joinType=[right])
       LogicalTableScan(table=[[CATALOG, SALES, EMP]])
       LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
   ```
   Now the Aggregate still has two fields (0 and 1), the first one (index 0) 
corresponds to the group 9, and the second one (index 1) corresponds to the 
argCall COUNT(DISTINCT $2). In this case, if we want to convert into the 
input's, we'll have 0 => 9, and 1 => 2; the current code will work fine only 
for the argCall conversion, but not for the group conversion.
   
   Notice I'm just applying here the same conversion that exists already e.g. 
when propagating the [RelMdColumnOrigins computation past an 
Aggregate](https://github.com/apache/calcite/blob/901aadbe8bd55e2da791d9d827185a8c803b1b76/core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnOrigins.java#L74)
 (in terms of our second example: "if we want to get origin of the column 0 of 
the Aggregate, we need to compute the origin of the column 9 of the Aggregate's 
input"). The auxiliary method `RelMdUtil.setAggChildKeys` (used by 
RelMdDistinctRowCount and RelMdPopulationSize for Aggregates) should behave in 
the same way as RelMdColumnOrigin for this conversion.



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

Reply via email to