Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23565 )
Change subject: KUDU-1261 [Java] Add spark bindings for Array columns ...................................................................... Patch Set 10: Code-Review+1 (4 comments) I took a quick look, overall I didn't notice any issues, but take this with a grain of salt: since I'm not expert in Scala, I'm not sure I can reliably spot problems, even if they are present. Adding +1 for a while. >From my side, I'll try to focus on addressing the test failure -- it seems >that's something related to modifying not-null bitset in ColumnDataView() http://gerrit.cloudera.org:8080/#/c/23565/10/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala File java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala: http://gerrit.cloudera.org:8080/#/c/23565/10/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala@270 PS10, Line 270: col.hasNestedTypeDescriptor What if this part isn't true when (colType == Type.NESTED): should an error be reported in such cases? http://gerrit.cloudera.org:8080/#/c/23565/10/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala@272 PS10, Line 272: if (nestedPB.hasArrayDescriptor) { What should be done in the 'else' case as of now, when only 1d-arrays are supported? http://gerrit.cloudera.org:8080/#/c/23565/10/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala: http://gerrit.cloudera.org:8080/#/c/23565/10/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/RowConverter.scala@169 PS10, Line 169: // Array write helper nit: add a summary what this helper does (just the essence)? http://gerrit.cloudera.org:8080/#/c/23565/10/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala: http://gerrit.cloudera.org:8080/#/c/23565/10/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/SparkUtil.scala@a129 PS10, Line 129: : : : : : : nit: I see this comment has been removed at least in PS10, isn't it relevant anymore? -- To view, visit http://gerrit.cloudera.org:8080/23565 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I786fdd1cbfbb67b4895b2e95b89addbc04341746 Gerrit-Change-Number: 23565 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-Comment-Date: Fri, 24 Oct 2025 03:44:35 +0000 Gerrit-HasComments: Yes
