Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24578 )
Change subject: KUDU-3800 Add diff-scan option to return unobservable rows ...................................................................... Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/24578/9/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/24578/9/src/kudu/client/client-test.cc@9688 PS9, Line 9688: ASSERT_FALSE(status.is_deleted); Shouldn't this be TRUE? http://gerrit.cloudera.org:8080/#/c/24578/9/src/kudu/client/client-test.cc@9727 PS9, Line 9727: ASSERT_FALSE(status.is_deleted); ditto http://gerrit.cloudera.org:8080/#/c/24578/9/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/24578/9/src/kudu/tserver/tablet_service.cc@3033 PS9, Line 3033: // Reject row_visibility=INCLUDE_UNOBSERVABLE unless this is a diff scan : // (i.e. snap_start_timestamp is set). Enforcing this uniformly here : // covers every read mode and avoids any silent-ignore path further down. : if (PREDICT_FALSE(scan_pb.row_visibility() == INCLUDE_UNOBSERVABLE && : !scan_pb.has_snap_start_timestamp())) { : *error_code = TabletServerErrorPB::INVALID_SCAN_SPEC; : return Status::InvalidArgument( : "row_visibility=INCLUDE_UNOBSERVABLE may only be set on a diff scan " : "(requires snap_start_timestamp)"); : } Here and line 3390-3396: Should these new rejection conditions have corresponding unit test that verify the error paths? -- To view, visit http://gerrit.cloudera.org:8080/24578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d0727fb2b89c727c669ec218e3bb8594336359a Gerrit-Change-Number: 24578 Gerrit-PatchSet: 9 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Tue, 18 Aug 2026 07:02:32 +0000 Gerrit-HasComments: Yes
