Skip to content

feat: Replace to_datum* functions with GenericDatumWriter#499

Open
Kriskras99 wants to merge 4 commits intomainfrom
feat/generic_datum_writer
Open

feat: Replace to_datum* functions with GenericDatumWriter#499
Kriskras99 wants to merge 4 commits intomainfrom
feat/generic_datum_writer

Conversation

@Kriskras99
Copy link
Contributor

This is the sibling to #496

value: &T,
) -> AvroResult<usize> {
let mut serializer =
SchemaAwareWriteSerializer::new(writer, self.schema, self.resolved.get_names(), None);
Copy link
Member

Choose a reason for hiding this comment

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

At many places we pass None as enclosing namespace.
We should use the schema's namespace instead.
The issue is that their types are different: Namespace (Option<String>) vs. NamespaceRef (Option<&str>)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that's right. In my mental model, the enclosing_namespace is the namespace of the parent schema of the schema currently being used.

Looking at the code for SchemaAwareWriteSerializer and Value::validate_internal this is exactly how it is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a fix for Value::validate_schemata that does not set the enclosing_namespace.

default added 2 commits March 4, 2026 09:41
`enclosing_namespace` is the namespace of the parent schema, but
there is no parent schema and should therefore not be set.
@Kriskras99 Kriskras99 force-pushed the feat/generic_datum_writer branch from 422a0e8 to f4f3fa5 Compare March 4, 2026 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants