PropertySchema properties are not JSON marshalled/unmarshalled about avro HOT 5 CLOSED

Proposal 1:

Sure, props can be included, but your proposed solution has an issue in that maps are not ordered and so the output of the JSON may not be constant (comparable as a string), which would not be great.

Proposal 2:

The way ocf.NewEncoder() handles the schema is a little confusing:

It is parsed as it needs a schema object to encode and to know the passed string is actually a schema. It is re-encoded to be sure there is nothing random in the initial string that could be a security issue.

.String() isn't the same as .MarshalJSON(), and I'm not sure what its purpose is. It seems the convention in this package is to look like JSON, but not completely.

.String() returns the canonical schema which is needed for schema resolution. While not specifically stated, it is the minimal form of schema needed for reading, which is why it was chosen.

I have no issue with using the JSON form as long as it is constant, but to not make this a breaking change, I think it should be configurable.

from avro.

Everything you said makes sense. Thank you for the link about "Parsing Canonical Form", it clarifies a lot. I understand why the JSON and canonical forms are not treated the same way.

your proposed solution has an issue in that maps are not ordered and so the output of the JSON may not be constant (comparable as a string), which would not be great.

If we continue using Parsing Canonical Form in ocf.NewEncoder(), then does deterministic JSON key ordering matter in the JSON form? If the difference is in unit tests, then I can adjust those.

The way ocf.NewEncoder() handles the schema is a little confusing:

It is parsed as it needs a schema object to encode and to know the passed string is actually a schema. It is re-encoded to be sure there is nothing random in the initial string that could be a security issue.

Makes sense.

.String() returns the canonical schema which is needed for schema resolution. While not specifically stated, it is the minimal form of schema needed for reading, which is why it was chosen.

This is interesting because the Apache Iceberg reference implementation (Java) generates Avro files that include doc attributes, which are explicitly named as to be stripped in Transforming into Parsing Canonical Form.

New Proposal 1

In JSON form, always emit all attributes, including properties, doc, aliases.

Do not worry about JSON field ordering in JSON form.

New Proposal 2

In Parsing Canonical Form, add properties to the output, and make the property output optional, defaulting to not including properties.

from avro.

In JSON form, always emit all attributes, including properties, doc, aliases.

This makes sense. I would still prefer this to be ordered.

In Parsing Canonical Form, add properties to the output

This is a no go. From the spec, this form cannot contain other fields other than those specified in [STRIP]. It also has implications on the schema fingerprint.

from avro.

I'll work on the JSON form proposal, with deterministic ordering.

from avro.

The merged PR is enough to satisfy my needs, as I've written code to write files without the ocf package. Thanks for your help, @nrwiersma

from avro.