vlsi commented on PR #5291: URL: https://github.com/apache/calcite/pull/5291#issuecomment-5820633773
The tests cover one variant of each thing this change touched. These cases are missing: 1. **Composite key.** The change reads the IN keys from `rex` instead of `joinInfo.leftKeys`, and the two differ once there is more than one key. Both new tests use a single column. For example: `select * from emp as t where (t.mgr, t.deptno) not in (select s.mgr, s.deptno from emp as s where s.empno = t.empno)`. 2. **The plan before decorrelation.** `testNotInCorrelatedNullable` only snapshots the decorrelated plan, so no reference file has the two nested `LogicalCorrelate` nodes that `translateIn` now has to handle. A `.withDecorrelate(false)` variant of the same query would add one. 3. **Other clauses than WHERE.** The plan test covers only WHERE, and the SELECT list is covered only in `sub-query.iq`. HAVING fails (see the inline comment on `bb.setRoot(join, false)`), and it needs a test once it is fixed: `select deptno from emp as e group by deptno having deptno not in (select s.mgr from emp as s where s.deptno = e.deptno)`. 4. **Two correlated `NOT IN` in one query.** `translateIn` casts `root` to `BiRel` and its left input to `Project`, so it depends on the shape of the tree. Two sub-queries chained one after the other put the most weight on that shape. For example: `select * from emp as t where t.mgr not in (select s.mgr from emp as s where s.empno = t.empno) and t.comm not in (select s.comm from emp as s where s.ename = t.ename)`. I checked that the plan is valid and that `expand=true` returns the same rows as `expand=false`, but no test guards 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]
