Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AVRO-3709: [Rust] Add aliases to record fields #2087

Merged
merged 4 commits into from
Feb 9, 2023

Conversation

martin-g
Copy link
Member

@martin-g martin-g commented Feb 8, 2023

AVRO-3709

What is the purpose of the change

Adds support for Record field aliases

Verifying this change

Old tests are updated.
New tests are added.

Documentation

  • Does this pull request introduce a new feature? yes
  • Rustdoc is updated

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
Use String instead of Alias.

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
@github-actions github-actions bot added the Rust label Feb 8, 2023
@martin-g
Copy link
Member Author

martin-g commented Feb 8, 2023

@woile Please review this PR and test the branch! Thank you!

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
@martin-g martin-g force-pushed the avro-3709-add-aliases-to-record-fields branch from fc39fae to a33f2db Compare February 8, 2023 12:41
@woile
Copy link
Contributor

woile commented Feb 8, 2023

Thank you Martin, I'll review tomorrow morning (CET)

.map(|alias| Alias::new(alias).expect("Alias is not valid"))
.collect::<Vec<Alias>>()
.map(|alias| alias.to_string())
.collect::<Vec<String>>()
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did you find the information about Aliases not requiring namespaces for RecordField? I was using the same alias parser to parse an Enum for example, but they now have different signature, it's not really a problem, but in the spec the text is the same for Enum and for RecordField

Copy link
Member Author

@martin-g martin-g Feb 9, 2023

Choose a reason for hiding this comment

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

Named schema's aliases are for the schema's name which might be namespaced.
Record field's aliases are for the field's name which is not namespaced. The field's type might be a (namespaced) reference to Schema.

@woile
Copy link
Contributor

woile commented Feb 9, 2023

It works on my project 👍🏻

@martin-g martin-g merged commit a512fa2 into master Feb 9, 2023
martin-g added a commit that referenced this pull request Feb 9, 2023
* AVRO-3709: [Rust] Add 'aliases' field to RecordField
* AVRO-3709: [Rust] Add support for serializing RecordField's aliases
* AVRO-3709: [Rust] Record field's aliases don't have namespace
Use String instead of Alias.
* AVRO-3709: [Rust] Add support for field aliases in avro_derive

---------

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
(cherry picked from commit a512fa2)
@martin-g martin-g deleted the avro-3709-add-aliases-to-record-fields branch February 9, 2023 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants