test(operator): add RIGHT OUTER, FULL OUTER, and NULL key join tests#49
test(operator): add RIGHT OUTER, FULL OUTER, and NULL key join tests#49
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 49 minutes and 15 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdded three new unit tests to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/operator_tests.cpp`:
- Around line 887-900: The test is currently skipping rows where both
tuple.get(0).is_null() and tuple.get(1).is_null(), which hides whether NULL keys
match; change the loop so it does not skip that case: when both are NULL, still
record the match (e.g., push a sentinel pair such as {INT64_MIN, INT64_MIN} or
otherwise record that a NULL/NULL row was found) instead of continuing, and then
update the expectations to assert the NULL/NULL match is present (e.g., expect
two results and verify one is the (1,1) row and the other indicates the
NULL/NULL match); use the existing symbols join->next(tuple),
tuple.get(...).is_null(), and tuple.get(...).to_int64() to implement this.
- Around line 797-809: The test only checks left nullability and not the actual
key values; update the assertions after the join->next(tuple) loop to validate
the actual right-side keys and the matched left value: when
tuple.get(0).is_null() is false assert tuple.get(0).to_int64() == 2 and
tuple.get(1).to_int64() == 2 for the matched row, and for the two unmatched rows
assert tuple.get(0).is_null() is true and that tuple.get(1).to_int64() equals 3
and 4 (in the expected order or by comparing a multiset if order is not
guaranteed); use the existing tuple.get(0) and tuple.get(1) calls to locate and
verify these values instead of only checking nullability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
poyrazK
force-pushed
the
feat/join-tests
branch
from
e2d16df to
c303c93
Compare
1 participant
Summary
HashJoinRightOuter: verify RIGHT JOIN outputs unmatched right rows with NULLsHashJoinFullOuter: verify FULL OUTER JOIN outputs unmatched rows on both sidesHashJoinNullKeys: document current NULL key matching behavior (non-standard SQL)Test plan
Summary by CodeRabbit