What happened?
Row#toString throws for an ITERABLE field whose value is a plain Iterable:
Schema schema =
Schema.builder().addStringField("k").addIterableField("vals", FieldType.STRING).build();
Iterable<String> bare = () -> Arrays.asList("p", "q").iterator();
Row.withSchema(schema).attachValues("k1", bare).toString();java.lang.IllegalArgumentException: value type is 'class ...' for field type 'ITERABLE'
at org.apache.beam.sdk.schemas.SchemaUtils.toPrettyFieldValueString(SchemaUtils.java:273)
toPrettyFieldValueString requires a List before iterating:
if (!(value instanceof List)) {
throw new IllegalArgumentException(...);
}An ITERABLE field declares an Iterable, so the guard is stricter than the type it is guarding. The branch below it only iterates and counts — both available from an Iterable once materialised.
Row#toString is SchemaUtils.toPrettyString(this), so one unusual field takes out logging and debugger output for every field beside it, which is a poor trade for a stricter check in a renderer.
Note on where the fix belongs
There is a reasonable position that a materialised Row should always hold a List for ARRAY/ITERABLE, and that a non-List arriving here means a producer is at fault. That may well be true and worth fixing separately — but the reproducer above uses only Schema.builder, Row.withSchema(...).attachValues(...) and toString(), so a value that is merely Iterable does reach the renderer through the public API, and a toString() that throws is hard to defend regardless of who produced the value.
Issue Priority
Priority: 3 (minor)
Issue Components
What happened?
Row#toStringthrows for anITERABLEfield whose value is a plainIterable:toPrettyFieldValueStringrequires aListbefore iterating:An
ITERABLEfield declares anIterable, so the guard is stricter than the type it is guarding. The branch below it only iterates and counts — both available from anIterableonce materialised.Row#toStringisSchemaUtils.toPrettyString(this), so one unusual field takes out logging and debugger output for every field beside it, which is a poor trade for a stricter check in a renderer.Note on where the fix belongs
There is a reasonable position that a materialised
Rowshould always hold aListforARRAY/ITERABLE, and that a non-Listarriving here means a producer is at fault. That may well be true and worth fixing separately — but the reproducer above uses onlySchema.builder,Row.withSchema(...).attachValues(...)andtoString(), so a value that is merelyIterabledoes reach the renderer through the public API, and atoString()that throws is hard to defend regardless of who produced the value.Issue Priority
Priority: 3 (minor)
Issue Components