[
https://issues.apache.org/jira/browse/AVRO-3248?focusedWorklogId=711176&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-711176
]
ASF GitHub Bot logged work on AVRO-3248:
----------------------------------------
Author: ASF GitHub Bot
Created on: 19/Jan/22 08:57
Start Date: 19/Jan/22 08:57
Worklog Time Spent: 10m
Work Description: martin-g commented on a change in pull request #1396:
URL: https://github.com/apache/avro/pull/1396#discussion_r787485297
##########
File path: lang/rust/src/types.rs
##########
@@ -703,13 +714,14 @@ impl Value {
fn resolve_union(self, schema: &UnionSchema) -> Result<Self, Error> {
let v = match self {
// Both are unions case.
- Value::Union(v) => *v,
+ Value::Union(_i, v) => *v,
// Reader is a union, but writer is not.
v => v,
};
// Find the first match in the reader schema.
- let (_, inner) =
schema.find_schema(&v).ok_or(Error::FindUnionVariant)?;
- Ok(Value::Union(Box::new(v.resolve(inner)?)))
+ // FIXME: this is wrong when the union consists of multiple same
records that have different names
Review comment:
True, but I wonder how big a problem it is ?
The name and doc might be wrong but the fields will be the same, so the
application logic should work.
##########
File path: lang/rust/src/de.rs
##########
@@ -162,7 +162,7 @@ impl<'de> de::EnumAccess<'de> for EnumDeserializer<'de> {
self.input.first().map_or(
Err(de::Error::custom("A record must have a least one field")),
|item| match (item.0.as_ref(), &item.1) {
- ("type", Value::String(x)) => Ok((
+ ("type", Value::String(x)) | ("type", Value::Enum(_, x)) =>
Ok((
Review comment:
This change does not look related to this PR.
@lulitao1997 Why did you make it ?
##########
File path: lang/rust/src/types.rs
##########
@@ -168,7 +173,11 @@ where
T: Into<Self>,
{
fn from(value: Option<T>) -> Self {
- Self::Union(Box::new(value.map_or_else(|| Self::Null, Into::into)))
+ // NOTE: this is incorrect in case first type in union is not "none"
Review comment:
This seems incorrect also when the union has more than two items.
And looks like a stopper for this PR.
--
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 711176)
Time Spent: 1h 40m (was: 1.5h)
> Rust: Support named types in UnionSchema
> ----------------------------------------
>
> Key: AVRO-3248
> URL: https://issues.apache.org/jira/browse/AVRO-3248
> Project: Apache Avro
> Issue Type: Improvement
> Reporter: Lu Litao
> Assignee: Martin Tzvetanov Grigorov
> Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> currently there's no support for named type in avro's Union type in rust.
> as stated in this comment of the UnionSchma struct
> {quote}// Used to ensure uniqueness of schema inputs, and provide constant
> time finding of the
> // schema index given a value.
> // **NOTE** that this approach does not work for named types, and will have
> to be modified
> // to support that. A simple solution is to also keep a mapping of the names
> used.
> {quote}
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
