rubenada commented on code in PR #3661:
URL: https://github.com/apache/calcite/pull/3661#discussion_r1473938944
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableBatchNestedLoopJoin.java:
##########
@@ -63,9 +64,11 @@ protected EnumerableBatchNestedLoopJoin(
RexNode condition,
Set<CorrelationId> variablesSet,
ImmutableBitSet requiredColumns,
- JoinRelType joinType) {
+ JoinRelType joinType,
+ Join originalJoin) {
super(cluster, traits, ImmutableList.of(), left, right, condition,
variablesSet, joinType);
this.requiredColumns = requiredColumns;
+ this.originalJoin = originalJoin;
}
public static EnumerableBatchNestedLoopJoin create(
Review Comment:
@JiajunBernoulli we could do that, but in that case if the deprecated
function is used, we will have an EnumerableBatchNestedLoopJoin without its
originalJoin (so originalJoin would be `null`), what shall we do in that
scenario for the RelMdRowCount computation of the EBNLJ?
The alternative would be, we do this as a breaking change to ensure we apply
the correction in all cases (we mention on the next release note that
"EnumerableBatchNestedLoopJoin now requires on its construction a new
parameter: the Join that originated it", tbd before merging the PR). And when
downstream projects upgrade, they'd need to adjust their code (if they create
EBNLJ on their own).
Both alternatives have pros and cons. Personally I prefer the second one,
but I'm open to discuss 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
