Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 81 additions & 58 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/codecs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ name = "generate-avro-fixtures"
path = "tests/bin/generate-avro-fixtures.rs"

[dependencies]
apache-avro = { version = "0.21.0", default-features = false }
apache-avro = { version = "0.22.0", default-features = false }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Document stricter Avro schema validation

With this version upgrade, schemas previously accepted by Vector but invalid under the Avro specification are rejected while building an Avro encoder or decoder, so affected configurations will fail to start after upgrading Vector. Because the change is user-observable and requires users to rewrite schemas such as the formerly accepted field-level array/map declarations, it needs a breaking changelog fragment rather than the proposed no-changelog treatment.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@omwbennett can you provide an example here? Trying to understand how big of a breaking change this is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I linked an old issue in the PR description, I've updated to link to the correct github issue + PR.

The core problem seems to be that avro-rs previously allowed defining complex types (e.g. array, enum etc.) directly in a field definition, when they should be inside a type block.
For example this was previously accepted by avro-rs:

{
  "type": "record",
  "name": "Test",
  "fields": [
    {"name": "enum_field", "type": "enum", "symbols": ["Spades", "Hearts", "Diamonds", "Clubs"]},
    {"name": "array_field", "type": "array", "items": "string"},
    {"name": "map_field", "type": "map", "values": "string"},
    {"name": "fixed_field", "type": "fixed", "size": 16}
  ]
}

but must now be:

{
  "type": "record",
  "name": "Test",
  "fields": [
    {"name": "enum_field", "type": {"type": "enum", "name": "Suit", "symbols": ["Spades", "Hearts", "Diamonds", "Clubs"]}},
    {"name": "array_field", "type": {"type": "array", "items": "string"}},
    {"name": "map_field", "type": {"type": "map", "values": "string"}},
    {"name": "fixed_field", "type": {"type": "fixed", "name": "FixedField", "size": 16}}
  ]
}

This brings it in line with the Python/Java SDKs, so it seems unlikely that users would be using these invalid schemas but I suppose it's still possible.

arrow = { version = "59.1.0", default-features = false, features = ["ipc", "json"], optional = true }
parquet = { version = "59.1.0", default-features = false, features = [
"arrow",
Expand Down
22 changes: 18 additions & 4 deletions lib/codecs/src/decoding/format/avro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,9 @@ impl Deserializer for AvroDeserializer {
bytes
};

let value = apache_avro::from_avro_datum(&self.schema, &mut bytes.reader(), None)?;
let value = apache_avro::reader::datum::GenericDatumReader::builder(&self.schema)
.build()?
.read_value(&mut bytes.reader())?;

let apache_avro::types::Value::Record(fields) = value else {
return Err(vector_common::Error::from("Expected an avro Record"));
Expand Down Expand Up @@ -280,7 +282,11 @@ mod tests {
message: "hello from avro".to_owned(),
};
let record_value = apache_avro::to_value(event).unwrap();
let record_datum = apache_avro::to_avro_datum(&schema, record_value).unwrap();
let record_datum = apache_avro::writer::datum::GenericDatumWriter::builder(&schema)
.build()
.unwrap()
.write_value_to_vec(record_value)
.unwrap();
let record_bytes = Bytes::from(record_datum);

let deserializer = AvroDeserializer::new(schema, false);
Expand All @@ -303,7 +309,11 @@ mod tests {
message: "hello from avro".to_owned(),
};
let record_value = apache_avro::to_value(event).unwrap();
let record_datum = apache_avro::to_avro_datum(&schema, record_value).unwrap();
let record_datum = apache_avro::writer::datum::GenericDatumWriter::builder(&schema)
.build()
.unwrap()
.write_value_to_vec(record_value)
.unwrap();

let mut bytes = BytesMut::new();
bytes.extend([0, 0, 0, 0, 0]); // 0 prefix + 4 byte schema id
Expand Down Expand Up @@ -331,7 +341,11 @@ mod tests {
};
let value = apache_avro::to_value(event).unwrap();
// let value = value.resolve(&schema).unwrap();
let datum = apache_avro::to_avro_datum(&schema, value).unwrap();
let datum = apache_avro::writer::datum::GenericDatumWriter::builder(&schema)
.build()
.unwrap()
.write_value_to_vec(value)
.unwrap();

let mut bytes = BytesMut::new();
bytes.extend([0, 0, 0, 0, 0]); // 0 prefix + 4 byte schema id
Expand Down
4 changes: 3 additions & 1 deletion lib/codecs/src/encoding/format/avro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ impl Encoder<Event> for AvroSerializer {
let log = event.into_log();
let value = apache_avro::to_value(log)?;
let value = value.resolve(&self.schema)?;
let bytes = apache_avro::to_avro_datum(&self.schema, value)?;
let writer = apache_avro::writer::datum::GenericDatumWriter::builder(&self.schema)
.build()?;
let bytes = writer.write_value_to_vec(value)?;
buffer.put_slice(&bytes);
Ok(())
}
Expand Down
11 changes: 6 additions & 5 deletions lib/codecs/tests/bin/generate-avro-fixtures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ fn generate_avro_test_case_fixed() -> Result<()> {
"type": "record",
"name": "test",
"fields": [
{"name": "fixed_field", "type":"fixed", "size": 16}
{"name": "fixed_field", "type": {"type": "fixed", "name": "fixed_field", "size": 16}}
]
}
"#;
Expand All @@ -166,7 +166,7 @@ fn generate_avro_test_case_enum() -> Result<()> {
"type": "record",
"name": "test",
"fields": [
{"name": "enum_field", "type": "enum", "symbols" : ["Spades", "Hearts", "Diamonds", "Clubs"]}
{"name": "enum_field", "type": {"type": "enum", "name": "Suit", "symbols": ["Spades", "Hearts", "Diamonds", "Clubs"]}}
]
}
"#;
Expand Down Expand Up @@ -217,7 +217,7 @@ fn generate_avro_test_case_array() -> Result<()> {
"type": "record",
"name": "test",
"fields": [
{"name": "array_field", "type": "array", "items" : "string"}
{"name": "array_field", "type": {"type": "array", "items": "string"}}
]
}
"#;
Expand All @@ -242,7 +242,7 @@ fn generate_avro_test_case_map() -> Result<()> {
"type": "record",
"name": "test",
"fields": [
{"name": "map_field", "type": "map", "values" : "long","default": {}}
{"name": "map_field", "type": {"type": "map", "values": "long"}, "default": {}}
]
}
"#;
Expand Down Expand Up @@ -472,7 +472,8 @@ fn generate_test_case_from_value(schema: &str, value: Value, filename: &str) ->
let schema = Schema::parse_str(schema)?;

let value = value.resolve(&schema)?;
let bytes = apache_avro::to_avro_datum(&schema, value)?;
let writer = apache_avro::writer::datum::GenericDatumWriter::builder(&schema).build()?;
let bytes = writer.write_value_to_vec(value)?;

let mut schema_file = File::create(format!("{FIXTURES_PATH}/{filename}.avsc"))?;
let mut avro_file = File::create(format!("{FIXTURES_PATH}/{filename}.avro"))?;
Expand Down
2 changes: 1 addition & 1 deletion lib/codecs/tests/data/avro/generated/array.avsc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading