chore(relational-table): store zero bytes for sentinel columns #2296
Conversation
| deserialize_datum_from(&DataType::Int64, &mut deserializer).unwrap() | ||
| ); | ||
| // Do not deserialize datum for SENTINEL_CELL_ID cuz the value length is 0. | ||
| if deserialize_column_id(&k[k.len() - 4..]).unwrap() != SENTINEL_CELL_ID { |
There was a problem hiding this comment.
I'm not sure the optimization is worth. We might need this check every time it's possible to get a sentinel? cc @skyzh
There was a problem hiding this comment.
In the future, we can use relational table API to get data. All tests should be refactored to use relational table API instead of getting a single k-v pairs.
Here I think a better way is to check if v is empty. Also should use deserialize_cell instead of deserialize_datum.
Codecov Report
@@ Coverage Diff @@
## main #2296 +/- ##
==========================================
- Coverage 71.15% 71.15% -0.01%
==========================================
Files 664 664
Lines 83660 83663 +3
==========================================
+ Hits 59526 59528 +2
- Misses 24134 24135 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
src/common/src/util/ordered/serde.rs
Outdated
| let value = serialize_cell(&None)?; | ||
| result.push((key, Some(value))); | ||
| // Store zero bytes for the sentinel value. | ||
| result.push((key, Some("".to_string().into_bytes()))); |
| deserialize_datum_from(&DataType::Int64, &mut deserializer).unwrap() | ||
| ); | ||
| // Do not deserialize datum for SENTINEL_CELL_ID cuz the value length is 0. | ||
| if deserialize_column_id(&k[k.len() - 4..]).unwrap() != SENTINEL_CELL_ID { |
There was a problem hiding this comment.
In the future, we can use relational table API to get data. All tests should be refactored to use relational table API instead of getting a single k-v pairs.
Here I think a better way is to check if v is empty. Also should use deserialize_cell instead of deserialize_datum.
BowenXiao1999
force-pushed
the
bw/use-zero-bytes-for-sentiel
branch
from
5f11ff8 to
e166e93
Compare
What's changed and what's your intention?
Store zero bytes for sentinel cell value.
Checklist
Refer to a related PR or issue link (optional)