libenchao commented on code in PR #2848:
URL: https://github.com/apache/calcite/pull/2848#discussion_r913762560


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimit.java:
##########
@@ -96,11 +96,7 @@ public static EnumerableLimit create(final RelNode input, 
@Nullable RexNode offs
     final BlockBuilder builder = new BlockBuilder();
     final EnumerableRel child = (EnumerableRel) getInput();
     final Result result = implementor.visitChild(this, 0, child, pref);
-    final PhysType physType =
-        PhysTypeImpl.of(
-            implementor.getTypeFactory(),
-            getRowType(),
-            result.format);
+    final PhysType physType = result.physType;

Review Comment:
   Yes, you are right. I also struggled with this change for some time.
   Without this change, `JdbcTest#testDynamicParameterInLimitOffset` will fail. 
The reason is that in this PR, we strengthened the ability to transform to semi 
join, and this test case is the affected one. The physical implementation for 
semi join and inner join is different in `EnumerableHashJoin`, and this leads 
to the test failure.
   
   I also thought about to leave it as a follow-up issue, but I found it's very 
hard to reproduce this error without this PR. Hence I put the fix in this PR.
   What do you think about this?



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