jordepic commented on code in PR #10713:
URL: https://github.com/apache/arrow-rs/pull/10713#discussion_r3833397754


##########
arrow-avro/src/reader/mod.rs:
##########
@@ -736,6 +741,31 @@ impl Decoder {
         Ok(total_consumed)
     }
 
+    /// Decode exactly one unframed Avro datum with the active writer schema.
+    ///
+    /// This is intended for transports such as Kafka where the message 
boundary is external to
+    /// Avro. It returns the number of datum bytes consumed, allowing the 
caller to ignore transport
+    /// payload bytes after the first datum when its format contract requires 
that behavior.
+    /// Consecutive unframed datums can be decoded by repeatedly passing the 
unconsumed suffix.
+    /// If the current batch is full, this method returns `Ok(0)` until 
[`Self::flush`] is called.
+    ///
+    /// The decoder must already have the desired active fingerprint, and this 
method does not
+    /// inspect or switch framing fingerprints.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the datum is incomplete, malformed, or 
incompatible with the active
+    /// writer schema.
+    pub fn decode_datum(&mut self, data: &[u8]) -> Result<usize, AvroError> {
+        if self.remaining_capacity == 0 {
+            return Ok(0);
+        }
+        let consumed = self.active_decoder.decode(data, 1)?;
+        self.remaining_capacity -= 1;
+        self.awaiting_body = false;
+        Ok(consumed)
+    }

Review Comment:
   Implemented in 2a5f2d925. The public decode_datum method is gone; 
ReaderBuilder::with_decoder_mode now selects DecoderMode::Framed (default) or 
DecoderMode::UnframedDatum at construction, and Decoder::decode dispatches 
internally. Updated the crate/module/builder docs, runnable example, README, 
and existing unframed tests to use the single decoding entry point.



##########
arrow-avro/src/reader/record.rs:
##########
@@ -717,9 +719,61 @@ impl Decoder {
                 inner.append_null()?;
             }
             Self::Union(u) => u.append_null()?,
-            Self::Nullable(_, null_buffer, inner) => {
+            Self::Nullable(_, null_buffer, _, pending_nulls) => {
                 null_buffer.append(false);
-                inner.append_null()?;
+                *pending_nulls += 1;
+            }
+        }
+        Ok(())
+    }
+
+    /// Append a run of null placeholders, deferring nullable children until 
their next value or
+    /// flush so sparse record subtrees can be materialized in bulk.
+    fn append_nulls(&mut self, count: usize) -> Result<(), AvroError> {
+        if count == 0 {
+            return Ok(());
+        }
+        match self {
+            Self::Null(size) => *size += count,
+            Self::Boolean(values) => values.append_n(count, false),
+            Self::Int32(values) | Self::Date32(values) | 
Self::TimeMillis(values) => {
+                values.resize(values.len() + count, 0)
+            }
+            Self::Int64(values)
+            | Self::Int32ToInt64(values)
+            | Self::TimeMicros(values)
+            | Self::TimestampMillis(_, values)
+            | Self::TimestampMicros(_, values)
+            | Self::TimestampNanos(_, values) => values.resize(values.len() + 
count, 0),
+            Self::Float32(values) | Self::Int32ToFloat32(values) | 
Self::Int64ToFloat32(values) => {
+                values.resize(values.len() + count, 0.0)
+            }
+            Self::Float64(values)
+            | Self::Int32ToFloat64(values)
+            | Self::Int64ToFloat64(values)
+            | Self::Float32ToFloat64(values) => values.resize(values.len() + 
count, 0.0),
+            Self::Binary(offsets, _)
+            | Self::String(offsets, _)
+            | Self::StringView(offsets, _)
+            | Self::BytesToString(offsets, _)
+            | Self::StringToBytes(offsets, _) => {
+                for _ in 0..count {
+                    offsets.push_length(0);
+                }
+            }
+            Self::Record(_, children, _, _) => {
+                for child in children {
+                    child.append_nulls(count)?;
+                }
+            }
+            Self::Nullable(_, null_buffer, _, pending_nulls) => {
+                null_buffer.append_n_nulls(count);
+                *pending_nulls += count;
+            }
+            other => {
+                for _ in 0..count {
+                    other.append_null()?;
+                }

Review Comment:
   Addressed in 2a5f2d925. append_nulls now matches Decoder exhaustively and 
has direct bulk arms for arrays/maps/strings with reserved offsets, fixed 
values, UUIDs, enums, all decimal widths, duration builders, custom 
primitive/temporal types, nested records, and run-end encoding. Union handling 
remains explicitly per-row where required, and append_null simply delegates to 
append_nulls(1).



##########
arrow-avro/src/reader/mod.rs:
##########
@@ -736,6 +741,31 @@ impl Decoder {
         Ok(total_consumed)
     }
 
+    /// Decode exactly one unframed Avro datum with the active writer schema.
+    ///
+    /// This is intended for transports such as Kafka where the message 
boundary is external to
+    /// Avro. It returns the number of datum bytes consumed, allowing the 
caller to ignore transport
+    /// payload bytes after the first datum when its format contract requires 
that behavior.
+    /// Consecutive unframed datums can be decoded by repeatedly passing the 
unconsumed suffix.
+    /// If the current batch is full, this method returns `Ok(0)` until 
[`Self::flush`] is called.
+    ///
+    /// The decoder must already have the desired active fingerprint, and this 
method does not
+    /// inspect or switch framing fingerprints.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the datum is incomplete, malformed, or 
incompatible with the active
+    /// writer schema.
+    pub fn decode_datum(&mut self, data: &[u8]) -> Result<usize, AvroError> {
+        if self.remaining_capacity == 0 {
+            return Ok(0);
+        }

Review Comment:
   Addressed in 2a5f2d925 with AvroError::BatchFull for unframed decoding when 
capacity is exhausted. A successful zero-width datum still returns Ok(0), and 
the new regression test covers both an empty record and a record containing 
only a null field, including flushing and decoding another row without 
duplication.



##########
arrow-avro/src/reader/record.rs:
##########
@@ -717,9 +719,61 @@ impl Decoder {
                 inner.append_null()?;
             }
             Self::Union(u) => u.append_null()?,
-            Self::Nullable(_, null_buffer, inner) => {
+            Self::Nullable(_, null_buffer, _, pending_nulls) => {

Review Comment:
   Implemented in 2a5f2d925. NullableDecoder::materialize_pending now 
bulk-materializes child placeholders and the validity suffix together, while 
null branches only increment pending_nulls. Non-null/default paths skip the 
bulk dispatcher when no nulls are pending and append validity only after the 
value succeeds. Added a regression test that verifies neither bitmap nor child 
values are touched during a 64-row null run.



##########
arrow-avro/src/reader/record.rs:
##########
@@ -276,7 +276,8 @@ enum Decoder {
     #[cfg(feature = "avro_custom_types")]
     RunEndEncoded(u8, usize, Box<Decoder>),
     Union(UnionDecoder),
-    Nullable(NullablePlan, NullBufferBuilder, Box<Decoder>),
+    /// Nullable value plus trailing null placeholders not yet materialized in 
the child decoder.
+    Nullable(NullablePlan, NullBufferBuilder, Box<Decoder>, usize),

Review Comment:
   Implemented in 2a5f2d925 with a named NullableDecoder containing plan, 
validity, values, and pending_nulls. Its materialize_pending method owns the 
shared invariant, and append_null, bulk append, defaults, decoding, and flush 
all transition through that representation consistently.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to