Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23441 )
Change subject: KUDU-1261 Fix bug in wire_protocol.cc ...................................................................... Patch Set 1: Code-Review+1 (2 comments) Thank you for the fix! http://gerrit.cloudera.org:8080/#/c/23441/1/src/kudu/common/wire_protocol.cc File src/kudu/common/wire_protocol.cc: PS1: I know this bug manifests itself in high-level/integration-level tests, but could you add a small unit test scenario into wire_protocol-test.cc that would fail without this patch? It might be something similar to ScalarArray1DToPB added earlier or just be a part of the scenario. That should help catch regressions without involving many other components in the middle. http://gerrit.cloudera.org:8080/#/c/23441/1/src/kudu/common/wire_protocol.cc@238 PS1, Line 238: attrs_target nit: rename into 'elem_type', and then re-use it under the 'if (type == DataType::NESTED)' clause without introducing and extra local variable in that scope? -- To view, visit http://gerrit.cloudera.org:8080/23441 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ef6d8490c3452cb87c22eb48c70263853892d11 Gerrit-Change-Number: 23441 Gerrit-PatchSet: 1 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 19 Sep 2025 00:23:07 +0000 Gerrit-HasComments: Yes
