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

Change subject: KUDU-1261 [Java] Add write support for Array Type
......................................................................


Patch Set 10:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java:

http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@60
PS10, Line 60:   /** Return the underlying FlatBuffer bytes exactly as passed 
in. */
             :   public byte[] toBytes() {
             :     return rawBytes;
             :   }
> Is it used anywhere?
Yes, in PartialRow.addObject() for passing ArrayCellView object directly.


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@73
PS10, Line 73: IndexOutOfBoundsException()
> Does it make sense to provide information on effective values of i and cont
Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@79
PS10, Line 79: Typed accessors
> nit: Types single element accessors
Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@86
PS10, Line 86: "Element " + i + " is NULL"
> here and elsewhere: is it possible to use a constant with the format of the
Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@111
PS10, Line 111: & 0xFF
> Looking into the UInt8Array.values(int j) implementation, I can see it alre
good catch. Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@132
PS10, Line 132:  & 0xFFFF
> Looking into the UInt16Array.values(int j) implementation, I can see it alr
Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@154
PS10, Line 154:  & 0xFFFFFFFFL
> Same here: UInt32Array.values(int j) already does this, so this isn't neede
Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@267
PS10, Line 267:  & 0xFF
> not needed?
Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@279
PS10, Line 279: & 0xFFFF
> not needed?
Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@291
PS10, Line 291: & 0xFFFFFFFFL
> not needed?
Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/ArrayCellView.java@348
PS10, Line 348: "Not a " + what + " array (tag=" + typeTag + ")"
> ScalarArray has name() method, so I'd rather see the expected name of the t
Yep, makes sense. Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:

http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1465
PS10, Line 1465:    * The accepted Object type is based on the column's {@link 
Type}:
> Should this be updated to reflect on the recent updates?
Done


http://gerrit.cloudera.org:8080/#/c/23482/10/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@1545
PS10, Line 1545: null
> How is it going to work if validity array is null?  Does it have some speci
Yes, this is handled internally. Added a comment to explain how it works.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icbe5e243eafe12a8977d40204dacab99624451eb
Gerrit-Change-Number: 23482
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Thu, 09 Oct 2025 05:24:49 +0000
Gerrit-HasComments: Yes

Reply via email to