fix: skip compression in create_per_value if compression metadata is set to none#5086
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5086 +/- ##
==========================================
- Coverage 81.72% 81.66% -0.07%
==========================================
Files 340 340
Lines 138875 138967 +92
Branches 138875 138967 +92
==========================================
- Hits 113502 113490 -12
- Misses 21633 21737 +104
Partials 3740 3740
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @jackye1995 @westonpace , could you help review when you have time, thanks very much~ |
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
One small nitpick but good to go otherwise
| let compressor = strategy.create_per_value(&field, &variable_data).unwrap(); | ||
| assert!(format!("{:?}", compressor).contains("VariableEncoder")); |
There was a problem hiding this comment.
I think the compression encodings would still have VariableEncoder in it (as an inner encoding). Maybe we can use extract_array_encoding_chain to get more accurate testing?
There was a problem hiding this comment.
Thanks your suggestion! Let me fix it.
wojiaodoubao
force-pushed
the
meta-none-compression
branch
4 times, most recently
from
de800e4 to
c85bb1a
Compare
wojiaodoubao
force-pushed
the
meta-none-compression
branch
from
c85bb1a to
d5276c3
Compare
|
Hi @westonpace , this pr is ready for review now, please help when you have time, thanks very much. |
…set to none (lance-format#5086) Currently, we can't disable compression in per-value encoding. This pr tries to fix it. --------- Co-authored-by: lijinglun <lijinglun@bytedance.com>
3 participants
Currently, we can't disable compression in per-value encoding. This pr tries to fix it.