Yuqi Du has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19369 )

Change subject: [tools] Limit to count of rows to dump accurately
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19369/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19369/2//COMMIT_MSG@8
PS2, Line 8:
           : The --nrows flag for 'local_replica dump rowset' doesn't
           : work if not set --dump_all_columns
If FLAGS_dump_all_columns is not enabled, then
https://gerrit.cloudera.org/c/19369/2/src/kudu/tools/tool_action_local_replica.cc#b1040~
 b1044 would dump the lines, right? If yes, the describe words should fix.

If FLAGS_dump_all_columns is enabled, then
https://gerrit.cloudera.org/c/19369/2/src/kudu/tools/tool_action_local_replica.cc#b1003,
 would work.


IMO, reason of the command not work is because   rows_left < 0 means unlimited, 
while
https://gerrit.cloudera.org/c/19369/2/src/kudu/tools/tool_action_local_replica.cc#b1046~
 b1048, L1047 will make
*row_left < 0.

So a simple method may be:

`
  if (*rows_left >= 0) {
    *rows_left -= limit;
  }
  *row_left = std::max<int64_t>(*rows_left, 0);
`


http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:

http://gerrit.cloudera.org:8080/#/c/19369/2/src/kudu/tools/tool_action_local_replica.cc@a1048
PS2, Line 1048:
It seems after the '}' add a line:
`
*row_left = std::max<int64_t>(*rows_left, 0);
`
May fix the bug.  If so, and if other changes has no benefit, they can be keep 
it as it is .



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia758ba910fccbbc06ac6c59a795574fb86d4e279
Gerrit-Change-Number: 19369
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Wed, 04 Jan 2023 09:56:02 +0000
Gerrit-HasComments: Yes

Reply via email to