feat: add the basic encode path for 2.1 #3002

westonpace · 2024-10-13T10:39:27Z

Adds a mini-block encoder
Adds "structural encoder" (2.1 concept) for struct and primitive
Adds compressor impl's for value compression

codecov-commenter · 2024-10-13T11:02:35Z

Codecov Report

Attention: Patch coverage is 19.35953% with 554 lines in your changes missing coverage. Please review.

Project coverage is 78.24%. Comparing base (6473f26) to head (2c4966f).

Files with missing lines	Patch %	Lines
.../lance-encoding/src/encodings/logical/primitive.rs	12.86%	235 Missing and 2 partials ⚠️
rust/lance-encoding/src/encoder.rs	4.27%	111 Missing and 1 partial ⚠️
...ust/lance-encoding/src/encodings/physical/value.rs	0.00%	95 Missing ⚠️
...ust/lance-encoding/src/encodings/logical/struct.rs	17.64%	56 Missing ⚠️
rust/lance-encoding/src/format.rs	0.00%	18 Missing ⚠️
rust/lance-encoding/src/decoder.rs	46.87%	17 Missing ⚠️
rust/lance-file/src/v2/reader.rs	77.77%	3 Missing and 3 partials ⚠️
rust/lance-file/src/v2/writer.rs	71.42%	4 Missing and 2 partials ⚠️
rust/lance-core/src/datatypes/field.rs	0.00%	3 Missing ⚠️
rust/lance-encoding/src/encodings/logical/list.rs	81.25%	2 Missing and 1 partial ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3002      +/-   ##
==========================================
- Coverage   78.97%   78.24%   -0.73%     
==========================================
  Files         238      238              
  Lines       75606    76234     +628     
  Branches    75606    76234     +628     
==========================================
- Hits        59707    59647      -60     
- Misses      12870    13553     +683     
- Partials     3029     3034       +5

Flag	Coverage Δ
unittests	`78.24% <19.35%> (-0.73%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

wjones127

Nice work! Some minor suggestions

wjones127 · 2024-10-14T17:00:13Z

python/src/file.rs

-        )
+                    )


Ah, thanks. Fixed.

wjones127 · 2024-10-14T17:01:14Z

rust/lance-encoding/src/decoder.rs

@@ -248,15 +248,51 @@ use crate::{BufferScheduler, EncodingsIo};
 // If users are getting batches over 10MiB large then it's time to reduce the batch size
 const BATCH_SIZE_BYTES_WARNING: u64 = 10 * 1024 * 1024;

+/// Top-levle encoding message for a page.  Wraps both the


Suggested change

/// Top-levle encoding message for a page. Wraps both the

/// Top-level encoding message for a page. Wraps both the

Good catch. Fixed.

wjones127 · 2024-10-14T17:10:11Z

rust/lance-encoding/src/encoder.rs

+                DataType::List(_child) | DataType::LargeList(_child) => {
+                    todo!()
+                }


Will this be in a follow up PR? Or isn't hit by this code path?

In a follow-up. I haven't done lists / repetition levels completely yet.

wjones127 · 2024-10-14T17:18:50Z

rust/lance-encoding/src/encodings/logical/primitive.rs

+                bits_per_value: 16,
+                num_values,
+            });
+            let levels_field = Field::new_arrow("", DataType::UInt16, false)?;


Is it worth pulling this out into a static?

I'll do that in the read PR

wjones127 · 2024-10-14T17:23:15Z

rust/lance-encoding/src/encodings/logical/struct.rs

+            .map(|child| child.finish(external_buffers))
+            .collect::<FuturesOrdered<_>>();
+        async move {
+            let mut encoded_columns = Vec::new();


Suggested change

let mut encoded_columns = Vec::new();

let mut encoded_columns = Vec::with_capacity(child_columns.len());

wjones127 · 2024-10-14T17:26:56Z

rust/lance-file/src/v2/writer.rs

+                return Err(Error::InvalidInput { source: format!("cannot write batch with {} rows because {} rows have already been written and Lance files cannot contain more than 2^32 rows", num_rows, self.rows_written).into(), location: location!() });
+            }


Isn't the limit u64::MAX?

Yes it is. Updated the message.

broccoliSpicy · 2024-10-14T18:40:27Z

rust/lance-encoding/src/encoder.rs

+/// Accessing this data will require 2 IOPS and accessing in a random-access fashion will require
+/// a repetition index.
+pub trait VariablePerValueCompressor: std::fmt::Debug + Send + Sync {
+    /// Compress the data into a single buffer where each value is encoded with the same number of bits


Suggested change

/// Compress the data into a single buffer where each value is encoded with the same number of bits

/// Compress the data into a single buffer where each value is encoded with different number of bits

Thanks! Updated the comment.

broccoliSpicy · 2024-10-14T19:20:37Z

rust/lance-encoding/src/encoder.rs

+    // For example, 1 would mean there are 2 values in the chunk and 12 would mean there
+    // are 4Ki values in the chunk.
+    //
+    // This must be <= 12 (i.e. <= 4096 values)


not sure this will be a problem here:

in compression algorithms like fastlanes bitpacking, when the input is less than 1024 values, the compression algorithm itself will pad the input size to 1024 values. the padded values are needed to write to disk and needed for decoding

That's fine. Compressors are free to pad as much as they want. The last block is allowed to have a non-power-of-two and, if it's less than 1024 values and fastlanes needs to pad then as long as it marks num_bytes correctly (to include the padding)

broccoliSpicy · 2024-10-14T20:18:33Z

rust/lance-encoding/src/encodings/logical/primitive.rs

+        }
+    }
+
+    fn encode_miniblock(


we may need to adjust the function signature here later when we want to do recursively encoding(arrays -> datablock, Result<EncodedPage> ->Result<DataBlock>) but I think it is fine now

I think only MiniBlockCompressor::compress will need recursion, not encode_miniblock but I do agree we still need to figure out that recursion still.

broccoliSpicy

excellent work!

rust/lance-encoding/src/encodings/physical/value.rs

Adds a mini-block encoder Adds structural encoder for struct and primitive Adds compressor impl's for value compression

…2 bytes rep/def. Clean up comments

westonpace · 2024-10-15T14:32:31Z

Going to merge on green so I can get the read path PR up (and this code isn't used yet anyways)

westonpace requested review from wjones127 and broccoliSpicy October 13, 2024 10:39

github-actions bot added enhancement New feature or request python labels Oct 13, 2024

westonpace mentioned this pull request Oct 13, 2024

feat: 2.1 encoding path #2989

Closed

wjones127 approved these changes Oct 14, 2024

View reviewed changes

broccoliSpicy reviewed Oct 14, 2024

View reviewed changes

broccoliSpicy approved these changes Oct 14, 2024

View reviewed changes

broccoliSpicy reviewed Oct 14, 2024

View reviewed changes

rust/lance-encoding/src/encodings/physical/value.rs Show resolved Hide resolved

westonpace added 3 commits October 15, 2024 07:07

Adds the basic encode path for 2.1

16eea24

Adds a mini-block encoder Adds structural encoder for struct and primitive Adds compressor impl's for value compression

change multiplier to 6 to make sure we handle even the worst case of …

b5d478c

…2 bytes rep/def. Clean up comments

Rebase, address pr comments

2c4966f

westonpace force-pushed the feat/2.1-encoding-path-2 branch from 8dd5819 to 2c4966f Compare October 15, 2024 14:29

broccoliSpicy merged commit f60a6ce into lancedb:main Oct 15, 2024
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add the basic encode path for 2.1 #3002

feat: add the basic encode path for 2.1 #3002

westonpace commented Oct 13, 2024

codecov-commenter commented Oct 13, 2024 •

edited

Loading

wjones127 left a comment

wjones127 Oct 14, 2024

westonpace Oct 15, 2024

wjones127 Oct 14, 2024

westonpace Oct 15, 2024

wjones127 Oct 14, 2024

westonpace Oct 15, 2024

wjones127 Oct 14, 2024

westonpace Oct 15, 2024

wjones127 Oct 14, 2024

westonpace Oct 15, 2024

wjones127 Oct 14, 2024

westonpace Oct 15, 2024

broccoliSpicy Oct 14, 2024

westonpace Oct 15, 2024

broccoliSpicy Oct 14, 2024

westonpace Oct 14, 2024

broccoliSpicy Oct 14, 2024

westonpace Oct 15, 2024

broccoliSpicy left a comment

westonpace commented Oct 15, 2024

	/// Top-levle encoding message for a page. Wraps both the
	/// Top-level encoding message for a page. Wraps both the

	let mut encoded_columns = Vec::new();
	let mut encoded_columns = Vec::with_capacity(child_columns.len());

		return Err(Error::InvalidInput { source: format!("cannot write batch with {} rows because {} rows have already been written and Lance files cannot contain more than 2^32 rows", num_rows, self.rows_written).into(), location: location!() });
		}

	/// Compress the data into a single buffer where each value is encoded with the same number of bits
	/// Compress the data into a single buffer where each value is encoded with different number of bits

feat: add the basic encode path for 2.1 #3002

feat: add the basic encode path for 2.1 #3002

Conversation

westonpace commented Oct 13, 2024

codecov-commenter commented Oct 13, 2024 • edited Loading

Codecov Report

wjones127 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

broccoliSpicy left a comment

Choose a reason for hiding this comment

westonpace commented Oct 15, 2024

codecov-commenter commented Oct 13, 2024 •

edited

Loading