jecsand838 commented on code in PR #8242:
URL: https://github.com/apache/arrow-rs/pull/8242#discussion_r2308912949
##########
arrow-avro/src/reader/mod.rs:
##########
@@ -202,30 +202,50 @@ impl Decoder {
}
// Attempt to handle a single‑object‑encoding prefix at the current
position.
- //
// * Ok(None) – buffer does not start with the prefix.
// * Ok(Some(0)) – prefix detected, but the buffer is too short; caller
should await more bytes.
// * Ok(Some(n)) – consumed `n > 0` bytes of a complete prefix (magic and
fingerprint).
+ #[inline(always)]
fn handle_prefix(&mut self, buf: &[u8]) -> Result<Option<usize>,
ArrowError> {
- // Need at least the magic bytes to decide (2 bytes).
- let Some(magic_bytes) = buf.get(..SINGLE_OBJECT_MAGIC.len()) else {
- return Ok(Some(0)); // Get more bytes
- };
- // Bail out early if the magic does not match.
- if magic_bytes != SINGLE_OBJECT_MAGIC {
- return Ok(None); // Continue to decode the next record
- }
- // Try to parse the fingerprint that follows the magic.
- let fingerprint_size = match self.fingerprint_algorithm {
- FingerprintAlgorithm::Rabin => self
- .handle_fingerprint(&buf[SINGLE_OBJECT_MAGIC.len()..], |bytes|
{
+ match self.fingerprint_algorithm {
+ FingerprintAlgorithm::Rabin => {
+ self.handle_prefix_common(buf, &SINGLE_OBJECT_MAGIC, |bytes| {
Fingerprint::Rabin(u64::from_le_bytes(bytes))
- })?,
- };
- // Convert the inner result into a “bytes consumed” count.
- // NOTE: Incomplete fingerprint consumes no bytes.
- let consumed = fingerprint_size.map_or(0, |n| n +
SINGLE_OBJECT_MAGIC.len());
- Ok(Some(consumed))
+ })
+ }
+ FingerprintAlgorithm::MD5 => {
+ self.handle_prefix_common(buf, &SINGLE_OBJECT_MAGIC, |bytes| {
+ Fingerprint::MD5(bytes)
+ })
+ }
+ FingerprintAlgorithm::SHA256 => {
+ self.handle_prefix_common(buf, &SINGLE_OBJECT_MAGIC, |bytes| {
+ Fingerprint::SHA256(bytes)
+ })
+ }
+ FingerprintAlgorithm::None => {
+ self.handle_prefix_common(buf, &CONFLUENT_MAGIC, |bytes| {
+ Fingerprint::Id(u32::from_be_bytes(bytes))
+ })
+ }
+ }
+ }
+
+ #[inline(always)]
+ fn handle_prefix_common<const MAGIC_LEN: usize, const N: usize>(
+ &mut self,
+ buf: &[u8],
+ magic: &[u8; MAGIC_LEN],
+ fingerprint_from: impl FnOnce([u8; N]) -> Fingerprint,
+ ) -> Result<Option<usize>, ArrowError> {
+ if buf.len() < MAGIC_LEN {
+ return Ok(Some(0));
+ }
+ if &buf[..MAGIC_LEN] != magic {
+ return Ok(None);
+ }
+ let consumed_fp = self.handle_fingerprint(&buf[MAGIC_LEN..],
fingerprint_from)?;
+ Ok(Some(consumed_fp.map_or(0, |n| n + MAGIC_LEN)))
Review Comment:
I'd recommend adding some of those comments back to keep the code cleaner
and easier to read.
```suggestion
// Need at least the magic bytes to decide
// 2 bytes for Avro Spec and 1 byte for Confluent Wire Protocol.
if buf.len() < MAGIC_LEN {
return Ok(Some(0));
}
// Bail out early if the magic does not match.
if &buf[..MAGIC_LEN] != magic {
return Ok(None);
}
// Try to parse the fingerprint that follows the magic.
let fingerprint_size = self.handle_fingerprint(&buf[MAGIC_LEN..],
fingerprint_from)?;
// Convert the inner result into a “bytes consumed” count.
// NOTE: Incomplete fingerprint consumes no bytes.
Ok(Some(fingerprint_size.map_or(0, |n| n + MAGIC_LEN)))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
