Fokko commented on code in PR #13445:
URL: https://github.com/apache/iceberg/pull/13445#discussion_r2299097640
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/data/SparkParquetWriters.java:
##########
@@ -613,13 +617,32 @@ private static class InternalRowWriter extends
ParquetValueWriters.StructWriter<
private final DataType[] types;
private InternalRowWriter(List<ParquetValueWriter<?>> writers,
List<DataType> types) {
- super(writers);
+ super(writerToFieldIndex(types, writers.size()), writers);
this.types = types.toArray(new DataType[0]);
}
@Override
protected Object get(InternalRow struct, int index) {
return struct.get(index, types[index]);
}
+
+ /** Returns a mapping from writer index to field index, skipping Unknown
columns. */
+ private static int[] writerToFieldIndex(List<DataType> types, int
numWriters) {
+ if (null == types) {
+ return IntStream.rangeClosed(0, numWriters).toArray();
Review Comment:
> should it be `range` or `rangeClosed`? I though the end should be
exclusive as `[0, numWriters)`.
I think it should be `range`, but I'd rather follow up on that in a separate
PR since there are also similar occurrences in other places.
> also if the types `list/array` is null, should we fail? is it a valid
scenario?
I don't think it can be `null`, but I think we should fail on `null ==
types`. To illustrate, this PR addresses Spark, but the Flink writers don't
pass in the struct, failing on UnknownTypes. I would also suggest this to be a
follow up PR.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
