kgyrtkirk commented on a change in pull request #1440: URL: https://github.com/apache/hive/pull/1440#discussion_r479342835
########## File path: ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveJoinConstraintsRule.java ########## @@ -213,61 +218,137 @@ public void onMatch(RelOptRuleCall call) { // 2) Check whether this join can be rewritten or removed RewritablePKFKJoinInfo r = HiveRelOptUtil.isRewritablePKFKJoin( - join, leftInput == fkInput, call.getMetadataQuery()); + join, fkInput, nonFkInput, call.getMetadataQuery()); // 3) If it is the only condition, we can trigger the rewriting if (r.rewritable) { - List<RexNode> nullableNodes = r.nullableNodes; - // If we reach here, we trigger the transform - if (mode == Mode.REMOVE) { - if (rightInputPotentialFK) { - // First, if FK is the right input, we need to shift - nullableNodes = nullableNodes.stream() - .map(node -> RexUtil.shift(node, 0, -leftInput.getRowType().getFieldCount())) - .collect(Collectors.toList()); - topProjExprs = topProjExprs.stream() - .map(node -> RexUtil.shift(node, 0, -leftInput.getRowType().getFieldCount())) - .collect(Collectors.toList()); - } - // Fix nullability in references to the input node - topProjExprs = HiveCalciteUtil.fixNullability(rexBuilder, topProjExprs, RelOptUtil.getFieldTypeList(fkInput.getRowType())); - // Trigger transformation - if (nullableNodes.isEmpty()) { - call.transformTo(call.builder() - .push(fkInput) - .project(topProjExprs) - .convert(project.getRowType(), false) - .build()); + rewrite(mode, fkInput, nonFkInput, join, topProjExprs, call, project, r.nullableNodes); + } else { + // check if FK side could be removed instead + + // Possibly this could be enhanced to take other join type into consideration. + if (joinType != JoinRelType.INNER) { + return; + } + + //first swap fk and non-fk input and see if we can rewrite them + RewritablePKFKJoinInfo fkRemoval = HiveRelOptUtil.isRewritablePKFKJoin( + join, nonFkInput, fkInput, call.getMetadataQuery()); + + if (fkRemoval.rewritable) { + // we have established that nonFkInput is FK, and fkInput is PK + // and there is no row filtering on FK side + + // check that FK side join column is distinct (i.e. have a group by) + ImmutableBitSet fkSideBitSet; + if (nonFkInput == leftInput) { + fkSideBitSet = leftBits; } else { - RexNode newFilterCond; - if (nullableNodes.size() == 1) { - newFilterCond = rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, nullableNodes.get(0)); - } else { - List<RexNode> isNotNullConds = new ArrayList<>(); - for (RexNode nullableNode : nullableNodes) { - isNotNullConds.add(rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, nullableNode)); + fkSideBitSet = rightBits; + } + + ImmutableBitSet.Builder fkJoinColBuilder = ImmutableBitSet.builder(); + for (RexNode conj : RelOptUtil.conjunctions(cond)) { + if (!conj.isA(SqlKind.EQUALS)) { + continue; Review comment: why do we skip all other kinds which are not `EQUALS`? I think instead there should be a return here instead of a continue ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org