zhoufek commented on code in PR #17477:
URL: https://github.com/apache/beam/pull/17477#discussion_r861128925
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/StaticSchemaInference.java:
##########
@@ -87,22 +88,33 @@ enum MethodType {
* public getter methods, or special annotations on the class.
*/
public static Schema schemaFromClass(
- Class<?> clazz, FieldValueTypeSupplier fieldValueTypeSupplier) {
+ Class<?> clazz,
+ FieldValueTypeSupplier fieldValueTypeSupplier,
+ HashSet<Class> alreadyVisitedSchemas) {
+ if (alreadyVisitedSchemas.contains(clazz)) {
+ throw new RuntimeException(
+ "Cannot infer schema with a circular reference. Class: " +
clazz.getTypeName());
+ }
+ alreadyVisitedSchemas.add(clazz);
Schema.Builder builder = Schema.builder();
for (FieldValueTypeInformation type : fieldValueTypeSupplier.get(clazz)) {
- Schema.FieldType fieldType = fieldFromType(type.getType(),
fieldValueTypeSupplier);
+ Schema.FieldType fieldType =
+ fieldFromType(type.getType(), fieldValueTypeSupplier,
alreadyVisitedSchemas);
if (type.isNullable()) {
builder.addNullableField(type.getName(), fieldType);
} else {
builder.addField(type.getName(), fieldType);
}
}
+ alreadyVisitedSchemas.remove(clazz);
Review Comment:
Simple case:
```
message Foo {
Bar bar = 1;
Baz baz = 2;
}
message Bar {
int32 i = 1;
}
message Baz {
Bar bar = 1;
}
```
There's no circular reference, so this is supported, but currently we
compute Bar's schema twice. The `Map` could help us look it up. I've also
updated the original example to show how this could be useful.
--
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]
