chore(codecs) update avro-rs to 0.22.0 - #26146
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d03d7415f0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| [[package]] | ||
| name = "aliasable" | ||
| version = "0.1.3" |
There was a problem hiding this comment.
The dependency upgrade adds shipped transitive crates including aliasable, ouroboros, ouroboros_macro, proc-macro2-diagnostics, and yansi, but LICENSE-3rdparty.csv is unchanged and contains none of them. This leaves the release's third-party attribution inventory incomplete; regenerate and commit it with make build-licenses.
AGENTS.md reference: AGENTS.md:L120-L126
Useful? React with 👍 / 👎.
| @@ -1 +1 @@ | |||
| {"type":"record","name":"test","fields":[{"name":"timestamp_millis_field","type":{"type":"long","logicalType":"timestamp-millis"}}]} No newline at end of file | |||
| {"type":"record","name":"test","fields":[{"name":"timestamp_millis_field","type":"long","logicalType":"timestamp-millis"}]} No newline at end of file | |||
There was a problem hiding this comment.
When the Avro fixture suite uses this regenerated schema, logicalType is now a sibling of the field's type rather than part of the type schema, so Avro treats it as field metadata and parses this value as a plain long. The same regression affects the other timestamp, time, and UUID fixtures, meaning their round-trip tests no longer exercise logical-type decoding or encoding; update the generator inputs to use nested type objects and regenerate the fixtures.
Useful? React with 👍 / 👎.
|
|
||
| [dependencies] | ||
| apache-avro = { version = "0.21.0", default-features = false } | ||
| apache-avro = { version = "0.22.0", default-features = false } |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
@omwbennett can you provide an example here? Trying to understand how big of a breaking change this is.
There was a problem hiding this comment.
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.
2 participants
Summary
Update to
avro-rs0.22.0 to enable us to useresolve_with_names()in #26112. Resolve a few breaking changes between 0.21.0 and 0.22.0 to get this working.References
Related: #26112
Vector configuration
N/A
How did you test this PR?
Existing tests pass. Re-ran
generate-avro-fixturesto update the syntax of Avro types in each fixture.Is this a breaking change?
Upgrading to 0.22.0 fixes apache/avro-rs#476 (See apache/avro-rs#479) which means avro-rs will no longer parse invalid Avro schemas. I suppose this could be considered a breaking change if anyone is currently using an invalid Avro schema?
Does this PR include user facing changes?
no-changeloglabel to this PR.Contributor Guidelines
@vectordotdev/vectorto reach out to us regarding this PR.pre-pushhook (template) or run the following locally before pushing:make fmtmake check-clippy(auto-fix withmake clippy-fix)make testgit merge origin masterandgit push.Cargo.lock), pleaserun
make build-licensesto regenerate the license inventory and commit the changes (if any). More details on the dd-rust-license-tool.