fix(sql): support unsigned ASOF join keys#19603
fix(sql): support unsigned ASOF join keys#19603sundy-li wants to merge 2 commits intodatabendlabs:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b940b91509
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
sundy-li
left a comment
There was a problem hiding this comment.
Found a blocking issue in the unsigned ASOF sentinel handling. Using the type-domain max/min as infinity() / ninfinity() makes the rewrite drop valid boundary matches (>= at MAX and <= at MIN). See the inline comment for a concrete example.
sundy-li
left a comment
There was a problem hiding this comment.
Focused verification passed, but the new unsigned sentinels still make ASOF results wrong at the legal type boundaries. rewrite_asof() uses infinity()/ninfinity() as the LEAD default and then applies strict < / > bounds, so rows at UInt*_MAX or UInt*_MIN cannot match the terminal or initial interval even though they should. This means unsigned ASOF joins are still incorrect for valid key values and need a different unbounded strategy or dedicated operator before this lands.
🤖 CI Job Analysis
📊 Summary
❌ NO RETRY NEEDEDAll failures appear to be code/test issues requiring manual fixes. 🔍 Job Details
🤖 AboutAutomated analysis using job annotations to distinguish infrastructure issues (auto-retried) from code/test issues (manual fixes needed). |
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
unwrap()calls withBadDataValueTypepropagation so unsupported types return an error instead of panickingFixes #19570
Tests
Validation:
cargo test -p databend-common-expression --test it test_integer_infinity_boundariescargo clippy -p databend-common-sql --lib --tests -- -D warningsType of change
This change is