Abhishek Chennaka has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23564 )

Change subject: KUDU-1261 [Java] Fix ColumnSchemaBuilder copy constructor
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23564/3/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
File java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java:

http://gerrit.cloudera.org:8080/#/c/23564/3/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@669
PS3, Line 669:         // The builder's `type` currently holds the element 
(scalar) type.
             :         if (this.type == Type.NESTED) {
             :           // cloned array => keep descriptor, do not reconstruct
             :           if (nestedTypeDescriptor == null || 
!nestedTypeDescriptor.isArray()) {
             :             throw new IllegalArgumentException(
             :                 "Builder.type is NESTED but no valid array 
descriptor found");
             :           }
             :           finalType = Type.NESTED;
             :         } else {
             :           // new array promotion
             :           ColumnSchema.ArrayTypeDescriptor arrDesc =
             :               new ColumnSchema.ArrayTypeDescriptor(this.type);
             :           nestedTypeDescriptor = 
ColumnSchema.NestedTypeDescriptor.forArray(arrDesc);
             :
> Ah, sorry -- I meant remove nestedTypeDescriptor from ColumnSchemaBuilder,
I see, yes that should be enough. Done.


http://gerrit.cloudera.org:8080/#/c/23564/4/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
File java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java:

http://gerrit.cloudera.org:8080/#/c/23564/4/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java@250
PS4, Line 250:     assertEquals("age", scalarCopy.getName());
             :     assertEquals(Type.INT32, scalarCopy.getType());
             :     assertTrue(scalarCopy.isKey());
             :     assertFalse(scalarCopy.isNullable());
             :     assertFalse("Scalar copy should not be marked as array", 
scalarCopy.isArray());
             :     assertEquals("scalar column", scalarCopy.getComment());
> Does it make sense to use assertEquals(scalarColCopy, scalarCol) here as we
Done



--
To view, visit http://gerrit.cloudera.org:8080/23564
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cdd3ec034c3118b476e7d0da264819b1d4c2cd8
Gerrit-Change-Number: 23564
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 22 Oct 2025 05:59:06 +0000
Gerrit-HasComments: Yes

Reply via email to