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 15: (6 comments) http://gerrit.cloudera.org:8080/#/c/23482/17/java/kudu-client/src/main/java/org/apache/kudu/client/Array1dSerdes.java File java/kudu-client/src/main/java/org/apache/kudu/client/Array1dSerdes.java: http://gerrit.cloudera.org:8080/#/c/23482/17/java/kudu-client/src/main/java/org/apache/kudu/client/Array1dSerdes.java@83 PS17, Line 83: > I'd suggest removing the code related to 'length' from this method: it does Yes, that is not needed. Added it while debugging a test case and didn't remove it after that. http://gerrit.cloudera.org:8080/#/c/23482/17/java/kudu-client/src/main/java/org/apache/kudu/client/Array1dSerdes.java@392 PS17, Line 392: > nit for here and below: indistinguishable from what? Added the details. indistinguishable from empty elements. http://gerrit.cloudera.org:8080/#/c/23482/17/java/kudu-client/src/main/java/org/apache/kudu/client/Array1dSerdes.java@491 PS17, Line 491: : : > nit: is it possible to use 'hasNulls()' here as well? No, as hasNulls() expects array of objects like byte[][], String[], BigDecimal[]. We could wrap it in an object but it checks the object itself and not the values in it. http://gerrit.cloudera.org:8080/#/c/23482/16/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/16/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@992 PS16, Line 992: , > nit: please avoid using non-ASCII characters in the code when it's possible Done http://gerrit.cloudera.org:8080/#/c/23482/16/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@992 PS16, Line 992: addVarLengthData(columnIndex, Array1dSerdes.serializeInt8(values, validity)); : } : : public void addArrayInt8(String columnName, byte[] values, boolean[] validity) { : addArrayInt8(schema.getColumnIndex(columnName), values, validity); : } > Ah, I guess you want to have this behavior as a convenience for String and Yes — that convenience mainly helps boxed and reference arrays (Integer[], String[], BigDecimal[], etc.) where nulls naturally signal invalids, so users can omit validity vectors. For primitive arrays, it just infers “all valid” since nulls aren’t possible. Overall, it simplifies API usage while still optimizing serialization by skipping all-true bitmaps. I'll document the corresponding methods accordingly. http://gerrit.cloudera.org:8080/#/c/23482/17/java/kudu-client/src/test/java/org/apache/kudu/client/TestArraySerdes.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestArraySerdes.java: http://gerrit.cloudera.org:8080/#/c/23482/17/java/kudu-client/src/test/java/org/apache/kudu/client/TestArraySerdes.java@377 PS17, Line 377: > Please also validate how it works with non-null empty validity array. Done -- 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: 15 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, 23 Oct 2025 01:19:56 +0000 Gerrit-HasComments: Yes
