search

risingwavelabs / risingwave

SQL stream processing, analytics, and management. We decouple storage and compute to offer instant failover, dynamic scaling, speedy bootstrapping, and efficient joins.
https://www.risingwave.com/slack
Apache License 2.0
6.59k stars 539 forks source link

bug: select a mv panic #16219

Open chenzl25 opened 3 months ago

chenzl25 commented 3 months ago

Describe the bug

create table t (a varchar);
create materialized view mv as select * from t order by a desc;
select * from mv where a < ''; -- panic

Error message/log

No response

To Reproduce

No response

Expected behavior

No response

How did you deploy RisingWave?

No response

The version of RisingWave

No response

Additional context

No response

hzxa21 commented 2 months ago

Per offline discussion, this is due to an overflow issue in https://github.com/risingwavelabs/risingwave/blob/c49bc37c1b59a0e6ed7aa860116547d302282256/src/storage/src/table/batch_table/storage_table.rs#L548-L562

The reason why storage table needs to convert exclusive start key to an inclusive key with next_key is because user key iterator doesn't support excluded start key . I think the reason why we didn't support excluded start key is just because the 1st version of user key iterator written 3yrs ago (how time flies!) only supports inclusive start key and we follow this assumption in the development.

In general, I think calculating next_key is risky so I think we can relax the inclusive start key requirement in user key iterator and support exclusive start key. The implementation can be straight-forward: for exclusive start key, we use start_key | MIN_EPOCH to construct the full key to seek and other logics remain unchanged.

hzxa21 commented 1 month ago

Related fix: #17018