You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2024/10/31 18:43:17 UTC
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22009
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
KUDU-3623 do not crash on invalid input for 'kudu table copy'
This patch adds code to handle invalid input for the --predicates flag
of the 'kudu table copy' tool. Now, instead of crashing on any
non-expected input, the tool exists gracefully and reports an actionable
error message. With that, now it's easier to understand what was wrong,
correct typos/mistakes, and re-run the tool again, eventually achieving
the desired goal.
A new test is added to cover the newly introduced functionality.
This is a follow-up to 0afeddf9e530762e0e47beb7428982763715c746.
Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
2 files changed, 263 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/22009/1
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 4: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/649/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 22:53:23 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Marton Greber, Yingchun Lai, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22009
to look at the new patch set (#2).
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
KUDU-3623 do not crash on invalid input for 'kudu table copy'
This patch adds code to handle invalid input for the --predicates flag
of the 'kudu table copy' tool. Now, instead of crashing on any
non-expected input, the tool exists gracefully and reports an actionable
error message. With that, now it's easier to understand what was wrong,
correct typos/mistakes, and re-run the tool again, eventually achieving
the desired goal.
A new test is added to cover the newly introduced functionality.
This is a follow-up to 0afeddf9e530762e0e47beb7428982763715c746.
Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
2 files changed, 280 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/22009/2
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 2:
Build Aborted
http://jenkins.kudu.apache.org/job/pre_commit/647/ : ABORTED
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 21:37:01 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/kudu-tool-test.cc@6114
PS1, Line 6114: just a single tablet server started
> nit: capitalize comment and add period at end.
Done
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc@287
PS1, Line 287: IsDouble
> I know it's not within the scope of this patch, but should this be IsFloat(
That's a good point.
I kept it as-is since I wasn't focusing on RapidJson-related peculiarities. However, after looking into the code a bit more closely, I think it's make sense to start using IsFloat() and GetFloat() here instead.
Thanks!
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc@318
PS1, Line 318: &kudu_value
> nit: error handling for this value?
Which error handling specifically? There is RETURN_NOT_OK around ParseValue() that handles errors of parsing and converting a JSON object into a KuduValue instance.
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc@469
PS1, Line 469: top-level predicate
> Maybe call this "predicate operator" to align with our documentation?
Done
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 21:37:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 4:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/649/
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 21:55:21 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Nov 2024 10:10:35 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 1:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/644/
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 31 Oct 2024 18:43:26 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Marton Greber, Yingchun Lai, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22009
to look at the new patch set (#3).
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
KUDU-3623 do not crash on invalid input for 'kudu table copy'
This patch adds code to handle invalid input for the --predicates flag
of the 'kudu table copy' tool. Now, instead of crashing on any
non-expected input, the tool exists gracefully and reports an actionable
error message. With that, now it's easier to understand what was wrong,
correct typos/mistakes, and re-run the tool again, eventually achieving
the desired goal.
A new test is added to cover the newly introduced functionality.
This is a follow-up to 0afeddf9e530762e0e47beb7428982763715c746.
Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
2 files changed, 280 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/22009/3
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/kudu-tool-test.cc@6114
PS1, Line 6114: just a single tablet server started
nit: capitalize comment and add period at end.
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc@287
PS1, Line 287: IsDouble
I know it's not within the scope of this patch, but should this be IsFloat()? I did take a look at the code and I don't see such a method, but I'm curious as to why we have different methods for int and int64 but use the same one for float and double.
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc@318
PS1, Line 318: &kudu_value
nit: error handling for this value?
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 20:18:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Marton Greber, Yingchun Lai, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22009
to look at the new patch set (#4).
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
KUDU-3623 do not crash on invalid input for 'kudu table copy'
This patch adds code to handle invalid input for the --predicates flag
of the 'kudu table copy' tool. Now, instead of crashing on any
non-expected input, the tool exists gracefully and reports an actionable
error message. With that, now it's easier to understand what was wrong,
correct typos/mistakes, and re-run the tool again, eventually achieving
the desired goal.
A new test is added to cover the newly introduced functionality.
This is a follow-up to 0afeddf9e530762e0e47beb7428982763715c746.
Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
2 files changed, 281 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/22009/4
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc@318
PS1, Line 318: &kudu_value
> Which error handling specifically? There is RETURN_NOT_OK around ParseValu
Took a closer look, looks like kudu_value will be set unless a Status that isn't OK is returned and that will be handled by RETURN_NOT_OK. So it looks good then, thanks.
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 21:44:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 3:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/648/
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 21:37:06 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 2:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/647/
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 21:33:46 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 1: Verified+1
Unrelated test failures (TSAN):
* maintenance_manager-test.0
* org.apache.kudu.client.TestSecurity, org.apache.kudu.client.TestKuduTransaction, org.apache.kudu.client.TestKuduClient
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 31 Oct 2024 20:49:04 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/644/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 31 Oct 2024 19:46:37 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 4: Verified+1
unrelated test failures
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 23:28:10 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc
File src/kudu/tools/table_scanner.cc:
http://gerrit.cloudera.org:8080/#/c/22009/1/src/kudu/tools/table_scanner.cc@469
PS1, Line 469: top-level predicate
Maybe call this "predicate operator" to align with our documentation?
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 20:54:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 3:
Build Aborted
http://jenkins.kudu.apache.org/job/pre_commit/648/ : ABORTED
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 21:55:12 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
KUDU-3623 do not crash on invalid input for 'kudu table copy'
This patch adds code to handle invalid input for the --predicates flag
of the 'kudu table copy' tool. Now, instead of crashing on any
non-expected input, the tool exists gracefully and reports an actionable
error message. With that, now it's easier to understand what was wrong,
correct typos/mistakes, and re-run the tool again, eventually achieving
the desired goal.
A new test is added to cover the newly introduced functionality.
This is a follow-up to 0afeddf9e530762e0e47beb7428982763715c746.
Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Reviewed-on: http://gerrit.cloudera.org:8080/22009
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Zoltan Chovan <zc...@cloudera.com>
Reviewed-by: Marton Greber <gr...@gmail.com>
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/table_scanner.cc
2 files changed, 281 insertions(+), 88 deletions(-)
Approvals:
Alexey Serbin: Verified
Zoltan Chovan: Looks good to me, but someone else must approve
Marton Greber: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] KUDU-3623 do not crash on invalid input for 'kudu table copy'
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/22009 )
Change subject: KUDU-3623 do not crash on invalid input for 'kudu table copy'
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/22009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96ee4c38f3074b44ce89ac1b18784058e9148daf
Gerrit-Change-Number: 22009
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Nov 2024 10:15:47 +0000
Gerrit-HasComments: No