martin-g commented on code in PR #447:
URL: https://github.com/apache/avro-rs/pull/447#discussion_r2734054788


##########
avro/src/writer.rs:
##########
@@ -195,9 +192,58 @@ impl<'a, W: Write> Writer<'a, W> {
     /// internal buffering for performance reasons. If you want to be sure the 
value has been
     /// written, then call [`flush`](Writer::flush).
     pub fn append_value_ref(&mut self, value: &Value) -> AvroResult<usize> {
+        if let Some(reason) = value.validate_internal(
+            self.schema,
+            self.resolved_schema.get_names(),
+            &self.schema.namespace(),
+        ) {
+            return Err(Details::ValidationWithReason {
+                value: value.clone(),
+                schema: self.schema.clone(),
+                reason,
+            }
+            .into());
+        }
+        self.unvalidated_append_value_ref(value)
+    }
+
+    /// Append a compatible value to a `Writer`.
+    ///
+    /// This function does **not** validate that the provided value matches 
the schema. If it does
+    /// not match, the file will contain corrupt data. Use 
[`Writer::append_value`] to have the

Review Comment:
   ```suggestion
       /// not match, the file will contain corrupt data. Use 
[`Writer::append`] to have the
   ```
   There is no `append_value()`. There are `append()` and `append_value_ref()`.
   
   Should this method be renamed to `unvalidated_append()`, for consistency ?
   Or maybe deprecate `append()` and delegate it to a new `append_value()` ?!



##########
avro/src/writer.rs:
##########
@@ -1805,4 +1828,32 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn avro_rs_310_append_unvalidated_value() -> TestResult {
+        let schema = Schema::String;
+        let value = Value::Int(1);
+
+        let mut writer = Writer::new(&schema, Vec::new())?;
+        writer.unvalidated_append_value_ref(&value)?;
+        writer.unvalidated_append_value(value)?;
+        let buffer = writer.into_inner()?;
+
+        assert_eq!(&buffer[buffer.len() - 18..buffer.len() - 16], &[2, 2]);

Review Comment:
   Please add a comment about the maths here - what are the magic numbers 18 
and 16.
   For easier maintenance in the future.



-- 
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