baeminbo commented on issue #36496: URL: https://github.com/apache/beam/issues/36496#issuecomment-3400753609
I believe the test failure caused by #36295 was fixed by #36489. #36295 set `SchemaFieldNumber` for some fields, but missed for fields recently added. Thus, while the fields annotated with `SchemaFieldNumber` had the index specified by the annotation, the new fields without `SchemaFieldNumber` had the index from the order in `AutoValueSchema` as follows: https://github.com/apache/beam/blob/2b9827b6c2da3806f1134cc4e08ac7130a611205/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/AutoValueSchema.java#L54-L69 https://github.com/apache/beam/blob/2b9827b6c2da3806f1134cc4e08ac7130a611205/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/FieldValueTypeInformation.java#L149-L155 Based on the error message, I believe `redistributeNumKeys` (without SchemaFieldNumber) was listed at the index 3 from `ReflectUtils.getMethods(Class)`, which conflicted with `confluentSchemaRegistrySubject` (`@SchemaFieldNumber("3")`). As #36489 put `SchemaFieldNumber` for the missed fields, all the fields had explicitly specified index, thus this test failure was fixed. Thank you, @aIbrahiim. ---- The concern with `sorted()` will be a problem of compatibility between a Row in old version and a Row in new version. Because the schemas for the two Rows are not compatible, thus can cause unexpected data corruption. However, if the sorted Row is not used for persisted states (such as for GroupByKey or states in StatefulDoFn), maybe it would be okay, IIUC. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
