[Python] Do not sort schema fields by default

[merlimat]

Motivation

In Avro schema, the order of fields is used in the validation process, so if we are sorting the fields, that will generate an unexpected schema for a python producer/consumer and it will make it not interoperable with Java and other clients.

[gaoran10]

Do we need to set this value to True to avoid compatibility problems?

[merlimat]

My only concern is that we'll keep getting users stuck with this issue if we don't fix the default behavior.

[gaoran10]

Maybe other users will meet new issues? This user is ok, but other users may be stuck.

[eolivelli]

When we updated Avro in 2.8.0 we saw the same thing happened on Java.

This is not a problem because the consumer has the original writer schema while reading.

This should not be a problem

@sijie can give more context, he gave me lot of useful advice during the Avro update

[hangc0276]

@merlimat Some tests run failed, Please take a look, thanks.

FAIL: test_avro_required_default (schema_test.SchemaTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/schema_test.py", line 856, in test_avro_required_default
    "default": None
AssertionError: {'fields': [{'default': None, 'type': ['null', 'int'], 'name': 'a'}, {'default': [truncated]... != {'fields': [{'default': None, 'type': ['null', 'int'], 'name': 'a'}, {'default': [truncated]...
Diff is 2313 characters long. Set self.maxDiff to None to see it.

======================================================================
FAIL: test_complex (schema_test.SchemaTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/schema_test.py", line 106, in test_complex
    "type": ["null", 'MySubRecord']
AssertionError: {'fields': [{'type': ['null', 'string'], 'name': 'a'}, {'type': ['null', {'field [truncated]... != {'fields': [{'type': ['null', 'string'], 'name': 'a'}, {'type': ['null', {'field [truncated]...
Diff is 1195 characters long. Set self.maxDiff to None to see it.

[sijie]

@eolivelli Putting the compatibility conversation aside, the PR here is valuable. Because it provides a flag to control whether to sort schema fields or not. So it will ensure python clients and java clients can generate the schema in the same order, which will improve debuggability and readability.

[eolivelli]

good for me

+1

[gaoran10]

If we don't think about the compatibility problem, I'm OK.

[hangc0276]

I found a case about the schema without sort field.
If we define the class like:

class T2(Record):
            b = Integer()
            a = Integer()
            d = String()
            c = Double()

But the schema fields of T2 without sort is:

[{'type': ['null', 'int'], 'name': 'a'}, {'type': ['null', 'double'], 'name': 'c'}, {'type': ['null', 'int'], 'name': 'b'}, {'type': ['null', 'string'], 'name': 'd'}]

However we expected order is the field define order of the class (I am not sure), such as b, a, d, c, but the output order is a, c, b, d

[codelipenghui]

@hangc0276 Could you please help create a separate issue to track the issue you have mentioned?

[hangc0276]

@hangc0276 Could you please help create a separate issue to track the issue you have mentioned?

@codelipenghui I create an issue to track it apache/pulsar-client-python#48

[BewareMyPower]

This PR actually leads to an incompatibility between the Java client and the Python client. See the issue here and the discussion in the mail list: https://lists.apache.org/thread/wl5rws7m0gqxc9n512llnpzf7kq5sp0j

In short, to be compatible with the POJO like:

class User {
    String name;
    int age;
    double score;
}

We have to give the following definition in Python:

class User(Record):
    _sorted_fields = True
    name = String()
    age = Integer(required=True)
    score = Double(required=True)

The default values are not compatible with the Java client so I suggest changing these default values. Please continue the discussion in the mail list if any of you has objection.

/cc @merlimat @gaoran10 @congbobo184 @codelipenghui @eolivelli @hangc0276 @mattisonchao

(BTW, these Python options are not documented well and I found the solution by reading the source code)