From 0651ba0ee14987cdd85acddf003139d057595f72 Mon Sep 17 00:00:00 2001 From: "Jorge C. Leitao" Date: Mon, 19 Sep 2022 04:17:44 +0000 Subject: [PATCH 1/2] Fixed error reading Avro list --- src/io/avro/read/deserialize.rs | 40 ++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/io/avro/read/deserialize.rs b/src/io/avro/read/deserialize.rs index 9fa9f152544..4434aacbda5 100644 --- a/src/io/avro/read/deserialize.rs +++ b/src/io/avro/read/deserialize.rs @@ -123,7 +123,16 @@ fn deserialize_value<'a>( .downcast_mut::>() .unwrap(); loop { - let len = util::zigzag_i64(&mut block)? as usize; + let len = util::zigzag_i64(&mut block)?; + let len = if len < 0 { + // Avro spec: If a block's count is negative, its absolute value is used, + // and the count is followed immediately by a long block size indicating the number of bytes in the block. This block size permits fast skipping through data, e.g., when projecting a record to a subset of its fields. + let _ = util::zigzag_i64(&mut block)?; + + -len + } else { + len + }; if len == 0 { break; @@ -340,14 +349,35 @@ fn skip_item<'a>(field: &Field, avro_field: &AvroSchema, mut block: &'a [u8]) -> }; loop { - let len = util::zigzag_i64(&mut block)? as usize; + let len = util::zigzag_i64(&mut block)?; + let (len, bytes) = if len < 0 { + // Avro spec: If a block's count is negative, its absolute value is used, + // and the count is followed immediately by a long block size indicating the number of bytes in the block. This block size permits fast skipping through data, e.g., when projecting a record to a subset of its fields. + let bytes = util::zigzag_i64(&mut block)?; + + (-len, Some(bytes)) + } else { + (len, None) + }; + + let bytes: Option = bytes + .map(|bytes| { + bytes + .try_into() + .map_err(|_| Error::oos("Avro block size negative or too large")) + }) + .transpose()?; if len == 0 { break; } - for _ in 0..len { - block = skip_item(inner, avro_inner, block)?; + if let Some(bytes) = bytes { + block = &block[bytes..]; + } else { + for _ in 0..len { + block = skip_item(inner, avro_inner, block)?; + } } } } @@ -488,7 +518,7 @@ pub fn deserialize( arrays .iter_mut() .zip(projection.iter()) - .filter_map(|x| if *x.1 { Some(x.0) } else { None }) + .filter_map(|x| x.1.then(|| x.0)) .map(|array| array.as_box()) .collect(), ) From b2fbadf31b479d0c71ad38b045c02080d47d1d38 Mon Sep 17 00:00:00 2001 From: "Jorge C. Leitao" Date: Tue, 20 Sep 2022 03:56:59 +0000 Subject: [PATCH 2/2] Fixed error --- src/io/avro/read/deserialize.rs | 6 ++- tests/it/io/avro/read.rs | 73 +++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/io/avro/read/deserialize.rs b/src/io/avro/read/deserialize.rs index 4434aacbda5..a9788ee8ca9 100644 --- a/src/io/avro/read/deserialize.rs +++ b/src/io/avro/read/deserialize.rs @@ -122,7 +122,9 @@ fn deserialize_value<'a>( .as_mut_any() .downcast_mut::>() .unwrap(); + // Arrays are encoded as a series of blocks. loop { + // Each block consists of a long count value, followed by that many array items. let len = util::zigzag_i64(&mut block)?; let len = if len < 0 { // Avro spec: If a block's count is negative, its absolute value is used, @@ -134,16 +136,18 @@ fn deserialize_value<'a>( len }; + // A block with count zero indicates the end of the array. if len == 0 { break; } + // Each item is encoded per the array’s item schema. let values = array.mut_values(); for _ in 0..len { block = deserialize_item(values, is_nullable, avro_inner, block)?; } - array.try_push_valid()?; } + array.try_push_valid()?; } DataType::Struct(inner_fields) => { let fields = match avro_field { diff --git a/tests/it/io/avro/read.rs b/tests/it/io/avro/read.rs index 5c301f7d1ba..e3e9eb04596 100644 --- a/tests/it/io/avro/read.rs +++ b/tests/it/io/avro/read.rs @@ -282,3 +282,76 @@ fn test_projected() -> Result<()> { } Ok(()) } + +fn schema_list() -> (AvroSchema, Schema) { + let raw_schema = r#" + { + "type": "record", + "name": "test", + "fields": [ + {"name": "h", "type": { + "type": "array", + "items": { + "name": "item", + "type": "int" + } + }} + ] + } +"#; + + let schema = Schema::from(vec![Field::new( + "h", + DataType::List(Box::new(Field::new("item", DataType::Int32, false))), + false, + )]); + + (AvroSchema::parse_str(raw_schema).unwrap(), schema) +} + +pub(super) fn data_list() -> Chunk> { + let data = [Some(vec![Some(1i32), Some(2), Some(3)]), Some(vec![])]; + + let mut array = MutableListArray::>::new_from( + Default::default(), + DataType::List(Box::new(Field::new("item", DataType::Int32, false))), + 0, + ); + array.try_extend(data).unwrap(); + + let columns = vec![array.into_box()]; + + Chunk::try_new(columns).unwrap() +} + +pub(super) fn write_list(codec: Codec) -> std::result::Result, avro_rs::Error> { + let (avro, _) = schema_list(); + // a writer needs a schema and something to write to + let mut writer = Writer::with_codec(&avro, Vec::new(), codec); + + // the Record type models our Record schema + let mut record = Record::new(writer.schema()).unwrap(); + record.put( + "h", + Value::Array(vec![Value::Int(1), Value::Int(2), Value::Int(3)]), + ); + writer.append(record)?; + + let mut record = Record::new(writer.schema()).unwrap(); + record.put("h", Value::Array(vec![])); + writer.append(record)?; + Ok(writer.into_inner().unwrap()) +} + +#[test] +fn test_list() -> Result<()> { + let avro = write_list(Codec::Null).unwrap(); + let expected = data_list(); + let (_, expected_schema) = schema_list(); + + let (result, schema) = read_avro(&avro, None)?; + + assert_eq!(schema, expected_schema); + assert_eq!(result, expected); + Ok(()) +}