This is an automated email from the ASF dual-hosted git repository.
mgrigorov pushed a commit to branch avro-3814/schema-resolution-union
in repository https://gitbox.apache.org/repos/asf/avro.git
The following commit(s) were added to
refs/heads/avro-3814/schema-resolution-union by this push:
new c21718a21 AVRO-3814: Revert changes to validate_internal()
c21718a21 is described below
commit c21718a216bd3ff92b8d0522a3368e940bcacf42
Author: Martin Tzvetanov Grigorov <[email protected]>
AuthorDate: Tue Aug 15 10:17:45 2023 +0300
AVRO-3814: Revert changes to validate_internal()
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
---
lang/rust/avro/src/schema.rs | 1 -
lang/rust/avro/src/types.rs | 102 ++++++++++++-------------------------------
lang/rust/avro/src/writer.rs | 10 +----
3 files changed, 29 insertions(+), 84 deletions(-)
diff --git a/lang/rust/avro/src/schema.rs b/lang/rust/avro/src/schema.rs
index 0c3ce3351..d688473bd 100644
--- a/lang/rust/avro/src/schema.rs
+++ b/lang/rust/avro/src/schema.rs
@@ -838,7 +838,6 @@ impl UnionSchema {
collected_names.extend(resolved_names);
let namespace = &schema.namespace().or_else(||
enclosing_namespace.clone());
- // Attempt to validate the value in order to ensure we've
selected the right schema.
value
.clone()
.resolve_internal(schema, &collected_names, namespace,
&None)
diff --git a/lang/rust/avro/src/types.rs b/lang/rust/avro/src/types.rs
index 2a812c3af..7e2c44e23 100644
--- a/lang/rust/avro/src/types.rs
+++ b/lang/rust/avro/src/types.rs
@@ -350,7 +350,7 @@ impl Value {
schemata.iter().any(|schema| {
let enclosing_namespace = schema.namespace();
- match self.validate_internal(schema, rs.get_names(),
&enclosing_namespace, false) {
+ match self.validate_internal(schema, rs.get_names(),
&enclosing_namespace) {
Some(reason) => {
let log_message = format!(
"Invalid value: {:?} for schema: {:?}. Reason: {}",
@@ -386,7 +386,6 @@ impl Value {
schema: &Schema,
names: &HashMap<Name, S>,
enclosing_namespace: &Namespace,
- schema_resolution: bool,
) -> Option<String> {
match (self, schema) {
(_, Schema::Ref { name }) => {
@@ -399,14 +398,7 @@ impl Value {
names.keys()
))
},
- |s| {
- self.validate_internal(
- s.borrow(),
- names,
- &name.namespace,
- schema_resolution,
- )
- },
+ |s| self.validate_internal(s.borrow(), names,
&name.namespace),
)
}
(&Value::Null, &Schema::Null) => None,
@@ -477,42 +469,24 @@ impl Value {
Schema::Enum(EnumSchema {
symbols, default, ..
}),
- ) => {
- if schema_resolution {
- // When resolving a schema the following rule applies:
- // if both are enums: if the writer’s symbol is not
present in the reader’s enum and the reader has a default value,
- // then that value is used, otherwise an error is
signalled.
- if symbols.contains(s) || default.is_some() {
- // If `s` is a symbol in the schema, or a default is
available then the value is valid.
- None
+ ) => symbols
+ .get(i as usize)
+ .map(|ref symbol| {
+ if symbol != &s {
+ Some(format!("Symbol '{s}' is not at position '{i}'"))
} else {
- Some(format!(
- "Unknown symbol '{s}': no symbol to fallback to is
available."
- ))
+ None
}
- } else {
- symbols
- .get(i as usize)
- .map(|ref symbol| {
- if symbol != &s {
- Some(format!("Symbol '{s}' is not at position
'{i}'"))
- } else {
- None
- }
- })
- .unwrap_or_else(|| match default {
- Some(_) => None,
- None => Some(format!("No symbol at position
'{i}'")),
- })
- }
- }
+ })
+ .unwrap_or_else(|| match default {
+ Some(_) => None,
+ None => Some(format!("No symbol at position '{i}'")),
+ }),
// (&Value::Union(None), &Schema::Union(_)) => None,
(&Value::Union(i, ref value), Schema::Union(inner)) => inner
.variants()
.get(i as usize)
- .map(|schema| {
- value.validate_internal(schema, names,
enclosing_namespace, schema_resolution)
- })
+ .map(|schema| value.validate_internal(schema, names,
enclosing_namespace))
.unwrap_or_else(|| Some(format!("No schema in the union at
position '{i}'"))),
(v, Schema::Union(inner)) => {
match inner.find_schema_with_known_schemata(v, Some(names),
enclosing_namespace) {
@@ -523,19 +497,14 @@ impl Value {
(Value::Array(items), Schema::Array(inner)) =>
items.iter().fold(None, |acc, item| {
Value::accumulate(
acc,
- item.validate_internal(inner, names, enclosing_namespace,
schema_resolution),
+ item.validate_internal(inner, names, enclosing_namespace),
)
}),
(Value::Map(items), Schema::Map(inner)) => {
items.iter().fold(None, |acc, (_, value)| {
Value::accumulate(
acc,
- value.validate_internal(
- inner,
- names,
- enclosing_namespace,
- schema_resolution,
- ),
+ value.validate_internal(inner, names,
enclosing_namespace),
)
})
}
@@ -558,7 +527,7 @@ impl Value {
record_fields.len(),
non_nullable_fields_count
));
- } else if record_fields.len() > fields.len() &&
!schema_resolution {
+ } else if record_fields.len() > fields.len() {
return Some(format!(
"The value's records length ({}) is greater than the
schema's ({} fields)",
record_fields.len(),
@@ -583,37 +552,20 @@ impl Value {
&field.schema,
names,
record_namespace,
- schema_resolution,
),
)
}
- None => {
- if schema_resolution {
- // While performing validation during
schema resolution we allow
- // extraneous fields to exist in the
Value::Record, as these
- // will get cleaned up later by the schema
resolution logic.
- acc
- } else {
- Value::accumulate(
- acc,
- Some(format!(
- "There is no schema field for
field '{field_name}'"
- )),
- )
- }
- }
+ None => Value::accumulate(
+ acc,
+ Some(format!("There is no schema field for
field '{field_name}'")),
+ ),
}
})
}
(Value::Map(items), Schema::Record(RecordSchema { fields, .. }))
=> {
fields.iter().fold(None, |acc, field| {
if let Some(item) = items.get(&field.name) {
- let res = item.validate_internal(
- &field.schema,
- names,
- enclosing_namespace,
- schema_resolution,
- );
+ let res = item.validate_internal(&field.schema, names,
enclosing_namespace);
Value::accumulate(acc, res)
} else if !field.is_nullable() {
Value::accumulate(
@@ -1305,7 +1257,7 @@ mod tests {
for (value, schema, valid, expected_err_message) in
value_schema_valid.into_iter() {
let err_message =
- value.validate_internal::<Schema>(&schema,
&HashMap::default(), &None, false);
+ value.validate_internal::<Schema>(&schema,
&HashMap::default(), &None);
assert_eq!(valid, err_message.is_none());
if !valid {
let full_err_message = format!(
@@ -1509,12 +1461,12 @@ mod tests {
("f".to_string(), Value::String("foo".to_string())), // extraneous
field
]);
assert!(value
- .validate_internal(&schema, rs.get_names(), &schema.namespace(),
false)
+ .validate_internal(&schema, rs.get_names(), &schema.namespace())
.is_some());
// However, when applying schema resolution rules, the extraneous
should be ignored by the validation logic.
- assert!(value
- .validate_internal(&schema, rs.get_names(), &schema.namespace(),
true)
- .is_none());
+ // assert!(value
+ // .validate_internal(&schema, rs.get_names(),
&schema.namespace(), true)
+ // .is_none());
let value = Value::Record(vec