-
Notifications
You must be signed in to change notification settings - Fork 613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(udf): eval_row panic due to misuse of input row as arg row #9240
Conversation
statement ok | ||
create materialized view mv as select dummy, hex_to_dec(v) from t; | ||
|
||
statement ok | ||
insert into t values ('2023-01-01', '1z'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the dummy column, it triggers panic:
thread 'risingwave-streaming-actor' panicked at 'Failed to append datum, array builder type: Utf8, scalar type: Date', src/common/src/array/mod.rs:562:1
Without the dummy column, it triggers panic:
thread 'risingwave-streaming-actor' panicked at 'itertools: .zip_eq() reached end of one iterator before the other', /Users/xiangjin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/itertools-0.10.5/src/zip_eq_impl.rs:48:13
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
It's really a problem that eval_row() can be hardly covered in normal SLT tests.
😢
Codecov Report
@@ Coverage Diff @@
## main #9240 +/- ##
==========================================
- Coverage 70.83% 70.82% -0.02%
==========================================
Files 1201 1201
Lines 200454 200472 +18
==========================================
- Hits 141992 141976 -16
- Misses 58462 58496 +34
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix. 🥵
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Fixes #9066
In this previous implementation, it converts an input row to an input chunk and reuses
eval
. However, the data types of columns are notarg_types
. We have to evaluate children expressions to get arrays / datums ofarg_types
.This always fails, but the
eval_row
code path executes rarely, mostly only during stream error tolerating, hash join condition, and frontend constant folding. Similar issues: #8115 (review) #7777Checklist For Contributors
./risedev check
(or alias,./risedev c
)Documentation