hsingh574 commented on code in PR #12855:
URL: https://github.com/apache/iceberg/pull/12855#discussion_r2067511721
##########
core/src/main/java/org/apache/iceberg/data/GenericRecord.java:
##########
@@ -65,13 +68,36 @@ private GenericRecord(StructType struct) {
this.nameToPos = NAME_MAP_CACHE.get(struct);
}
- private GenericRecord(GenericRecord toCopy) {
+ private GenericRecord(GenericRecord toCopy, boolean deepCopyValues) {
this.struct = toCopy.struct;
this.size = toCopy.size;
- this.values = Arrays.copyOf(toCopy.values, toCopy.values.length);
+ if (deepCopyValues) {
+ this.values = new Object[toCopy.values.length];
+ for (int i = 0; i < toCopy.values.length; i++) {
+ this.values[i] = deepCopyValue(this.struct.fields().get(i).type(),
toCopy.values[i]);
+ }
+ } else {
+ this.values = Arrays.copyOf(toCopy.values, toCopy.values.length);
+ }
this.nameToPos = toCopy.nameToPos;
}
+ private Object deepCopyValue(Type type, Object value) {
Review Comment:
I believe the unit tests I added for binary and struct types in the
DeleteReadTests class are using the generics implementation
https://github.com/apache/iceberg/blob/242717c8c516dcc49bc4368b77cd3f3af40720c8/data/src/test/java/org/apache/iceberg/data/TestGenericReaderDeletes.java#L51
For list and map using a similar setup I was running into exceptions like:
```
org.apache.iceberg.exceptions.ValidationException: Invalid schema: multiple
fields for name mapData.key: 7 and 7
```
which looks like its coming from ids of list map types not being projected
https://github.com/apache/iceberg/blob/242717c8c516dcc49bc4368b77cd3f3af40720c8/api/src/main/java/org/apache/iceberg/types/GetProjectedIds.java#L49
which leads to `missingIds` being containing the map/list column id
http://github.com/apache/iceberg/blob/242717c8c516dcc49bc4368b77cd3f3af40720c8/data/src/main/java/org/apache/iceberg/data/DeleteFilter.java#L286
and Schema creation failing because the column id was added twice
https://github.com/apache/iceberg/blob/242717c8c516dcc49bc4368b77cd3f3af40720c8/data/src/main/java/org/apache/iceberg/data/DeleteFilter.java#L317
Am I missing something or would equality deletes for map/list not work due
to this constraint?
Anyways, I'll scope the change to ByteBuffer and GenericRecord types for now.
--
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]
