vlsi commented on a change in pull request #2194:
URL: https://github.com/apache/calcite/pull/2194#discussion_r501514402
##########
File path: core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
##########
@@ -3132,6 +3133,35 @@ private void checkNodeTypeCount(String sql, Map<Class<?
extends RelNode>, Intege
is("=($0, $1)"));
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-4315";>[CALCITE-4315]
+ * NPE in RelMdUtil#checkInputForCollationAndLimit</a>. */
+ @Test void testCheckInputForCollationAndLimit() {
+ final Project rel = (Project) convertSql("select * from emp, dept");
+ final Join join = (Join) rel.getInput();
+ final RelOptTable empTable = join.getInput(0).getTable();
+ final RelOptTable deptTable = join.getInput(1).getTable();
+ Frameworks.withPlanner((cluster, relOptSchema, rootSchema) -> {
+ checkInputForCollationAndLimit(cluster, empTable, deptTable);
+ return null;
+ });
+ }
+
+ private void checkInputForCollationAndLimit(RelOptCluster cluster,
RelOptTable empTable,
+ RelOptTable deptTable) {
+ final RexBuilder rexBuilder = cluster.getRexBuilder();
+ final RelMetadataQuery mq = cluster.getMetadataQuery();
+ final List<RelHint> hints = ImmutableList.of();
+ final LogicalTableScan empScan = LogicalTableScan.create(cluster,
empTable, hints);
+ final LogicalTableScan deptScan = LogicalTableScan.create(cluster,
deptTable, hints);
+ final LogicalJoin join =
+ LogicalJoin.create(empScan, deptScan, ImmutableList.of(),
+ rexBuilder.makeLiteral(true), ImmutableSet.of(),
JoinRelType.INNER);
+ assertTrue(
+ RelMdUtil.checkInputForCollationAndLimit(mq, join,
join.getTraitSet().getCollation(),
+ null, null));
Review comment:
It might be fine if you copy your explanation to the assert message: `()
-> "we are checking the join against its own collation, fetch=null, offset=null
=> checkInputForCollationAndLimit must be true. join=" + join)`
WDYT?
>for this test I can perfectly remove the assertTrue(...), and just the call
checkInputForCollationAndLimit.
If you remove all the asserts, then the test does not add much value. The
code won't compile, and it would force the developer to handle null.
----------------------------------------------------------------
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:
[email protected]
