[GitHub] [hive] kgyrtkirk commented on a change in pull request #1432: [WIP] HIVE-24072 aggjoin mapping

[GitBox <gi...@apache.org>]

kgyrtkirk commented on a change in pull request #1432:
URL: https://github.com/apache/hive/pull/1432#discussion_r478508344



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveAggregateJoinTransposeRule.java
##########
@@ -145,8 +145,7 @@ public void onMatch(RelOptRuleCall call) {
         int fieldCount = joinInput.getRowType().getFieldCount();
         final ImmutableBitSet fieldSet =
             ImmutableBitSet.range(offset, offset + fieldCount);
-        final ImmutableBitSet belowAggregateKeyNotShifted =
-            belowAggregateColumns.intersect(fieldSet);

Review comment:
       @jcamachor 
   I think this intersect is not needed; it cause some trouble in case
   * grouped by say 1,3,4
   * columns 1,2,3 is coming from left side of join; column 2 is the join key
   * the intersect will remove column 2 ; which will induce a shift in the mapping - and will rotate in the join key into a projkect later on
   
   I think in calcite the following is true:
   for a join betwen inputs: 
   * T1(a,b,c)
   * T2(d,e,f)
   the output is always a,b,c,d,e,f - independently the join condition
   so I see no rational need for this `intersect`...it was here for a few years now...and the class "borned" with this line in 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org