dramaticlly commented on code in PR #18342:
URL: https://github.com/apache/iceberg/pull/18342#discussion_r4159187846
##########
api/src/main/java/org/apache/iceberg/util/StructProjection.java:
##########
@@ -223,4 +223,19 @@ public <T> T get(int pos, Class<T> javaClass) {
public <T> void set(int pos, T value) {
throw new UnsupportedOperationException("Cannot set fields in a
TypeProjection");
}
+
+ @Override
+ public String toString() {
+ StringBuilder sb = new StringBuilder();
+ sb.append("StructProjection{");
+ List<Types.NestedField> fields = type.fields();
+ for (int i = 0; i < fields.size(); i += 1) {
+ if (i > 0) {
+ sb.append(", ");
+ }
+ sb.append(fields.get(i).name()).append("=").append(get(i, Object.class));
+ }
+ sb.append("}");
+ return sb.toString();
Review Comment:
Looks like we already used StringJoiner before in PartitionSet
https://github.com/apache/iceberg/blob/574e44670b8502479b5c74d0edf0b9190d235af4/core/src/main/java/org/apache/iceberg/util/PartitionSet.java#L194,
so it shall save some manual stringBuilder logic. Might worth considering
something like below
```suggestion
List<Types.NestedField> fields = type.fields();
StringJoiner joiner = new StringJoiner(", ", "StructProjection{", "}");
for (int pos = 0; pos < fields.size(); pos += 1) {
joiner.add(fields.get(pos).name() + "=" + get(pos, Object.class));
}
return joiner.toString();
```
On a separate note, I noticed that if we use get(pos, Object.class) on
deeply nested struct, it might have side effect of invoking
`javaClass.cast(nestedProjections[pos].wrap(nestedStruct))`, so worth double
checking whether we want the behavior as part of toString call.
##########
core/src/main/java/org/apache/iceberg/util/StructLikeUtil.java:
##########
@@ -64,5 +64,19 @@ public <T> T get(int pos, Class<T> javaClass) {
public <T> void set(int pos, T value) {
throw new UnsupportedOperationException("Struct copy cannot be
modified");
}
+
+ @Override
+ public String toString() {
+ StringBuilder sb = new StringBuilder();
+ sb.append("[");
+ for (int i = 0; i < values.length; i += 1) {
+ if (i > 0) {
+ sb.append(", ");
+ }
+ sb.append(values[i]);
+ }
+ sb.append("]");
+ return sb.toString();
Review Comment:
This is similar to
[`StructTransform::toString`](https://github.com/apache/iceberg/blob/574e44670b8502479b5c74d0edf0b9190d235af4/api/src/main/java/org/apache/iceberg/StructTransform.java#L98-L109)
with values is already object array, so can probably simplify to
```suggestion
return Arrays.stream(values)
.map(String::valueOf)
.collect(Collectors.joining(", ", "[", "]"));
```
--
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]
