Skip to content

Commit 88f6333

Browse files
authored
[HZ-5348] Sort fields in compact schema by name (#1577)
Fixes #1576
1 parent 5031056 commit 88f6333

4 files changed

Lines changed: 31 additions & 8 deletions

File tree

src/serialization/compact/Schema.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,13 @@ export class Schema {
3737
constructor(typeName: string, fields: FieldDescriptor[]) {
3838
this.typeName = typeName;
3939
this.fields = fields;
40-
this.fieldDefinitionMap = new Map<string, FieldDescriptor>();
40+
this.fields.sort((field1, field2) => {
41+
return field1.fieldName > field2.fieldName ? 1 : -1;
42+
})
4143

42-
for (const field of fields) {
44+
this.fieldDefinitionMap = new Map<string, FieldDescriptor>();
45+
for (const field of this.fields) {
46+
// map entries are sorted by insertion order
4347
this.fieldDefinitionMap.set(field.fieldName, field);
4448
}
4549

@@ -101,7 +105,7 @@ export class Schema {
101105
getFields() : IterableIterator<FieldDescriptor> {
102106
return this.fieldDefinitionMap.values();
103107
}
104-
108+
105109
private hasSameFields(other: Schema): boolean {
106110
if (other.fieldDefinitionMap.size !== this.fieldDefinitionMap.size) {
107111
return false;

src/serialization/compact/SchemaWriter.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,14 +212,12 @@ export class SchemaWriter implements CompactWriter {
212212
'Field with the name ' + field.fieldName + ' already exists'
213213
);
214214
}
215-
215+
216216
this.fieldNames.add(field.fieldName);
217217
this.fields.push(field);
218218
}
219219

220220
build() : Schema {
221-
return new Schema(this.typeName, this.fields.sort((field1, field2) => {
222-
return field1.fieldName > field2.fieldName ? 1 : -1;
223-
}));
221+
return new Schema(this.typeName, this.fields);
224222
}
225223
}

test/unit/serialization/compact/RabinFingerprintTest.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ const chai = require('chai');
1919
const Long = require('long');
2020
const { RabinFingerprint64, INIT } = require('../../../../lib/serialization/compact/RabinFingerprint');
2121
const { SchemaWriter } = require('../../../../lib/serialization/compact/SchemaWriter');
22+
const {Schema} = require('../../../../lib/serialization/compact/Schema');
23+
const {FieldDescriptor} = require('../../../../lib/serialization/generic_record/FieldDescriptor');
24+
const {FieldKind} = require('../../../../lib/serialization/generic_record/FieldKind');
25+
2226
chai.should();
2327

2428
describe('RabinFingerprintTest', function () {
@@ -88,4 +92,19 @@ describe('RabinFingerprintTest', function () {
8892
const schema = writer.build();
8993
schema.schemaId.eq(Long.fromString('3662264393229655598')).should.be.true;
9094
});
95+
96+
it('should compute the same fingerprint for equivalent schemas regardless of the field order', function () {
97+
const schema1 = new Schema('my-schema', [
98+
new FieldDescriptor('field-1', FieldKind.ARRAY_OF_DATE),
99+
new FieldDescriptor('field-2', FieldKind.ARRAY_OF_DATE),
100+
]);
101+
const fp1 = RabinFingerprint64.ofSchema(schema1);
102+
103+
const schema2 = new Schema('my-schema', [
104+
new FieldDescriptor('field-2', FieldKind.ARRAY_OF_DATE),
105+
new FieldDescriptor('field-1', FieldKind.ARRAY_OF_DATE),
106+
]);
107+
const fp2 = RabinFingerprint64.ofSchema(schema2);
108+
fp1.eq(fp2).should.be.true;
109+
});
91110
});

test/unit/serialization/compact/SchemaTest.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ describe('SchemaTest', function () {
2727
const boolCount = 100;
2828
const boolFields = new Array(100);
2929
for (let i = 0; i < boolCount; i++) {
30-
boolFields[i] = new FieldDescriptor(i.toString(), FieldKind.BOOLEAN);
30+
// the fields are sorted by name, so have to append 0 for numbers < 10
31+
const name = i >= 10? i.toString() : '0' + i.toString();
32+
boolFields[i] = new FieldDescriptor(name, FieldKind.BOOLEAN);
3133
}
3234

3335
const fields = [

0 commit comments

Comments
 (0)