Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/7805 )
Change subject: IMPALA-5425: Add test for validating input when setting query options ...................................................................... Patch Set 12: (2 comments) http://gerrit.cloudera.org:8080/#/c/7805/12/be/src/service/query-options-test.cc File be/src/service/query-options-test.cc: http://gerrit.cloudera.org:8080/#/c/7805/12/be/src/service/query-options-test.cc@70 PS12, Line 70: if 'str' would be successfully read as expected : // value. I don't understand that. Maybe this should say: Create a shortcut function to verify that the given query option has the 'expected_value' value after setting the 'option_def' option to 'str'. or something like that? http://gerrit.cloudera.org:8080/#/c/7805/12/be/src/service/query-options-test.cc@86 PS12, Line 86: // Create a shortcut function to test if 'str' would result into a failure. I think that should be reworded as per above. -- To view, visit http://gerrit.cloudera.org:8080/7805 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I510e02bb0776673d8cbfc22b903831882c6908d7 Gerrit-Change-Number: 7805 Gerrit-PatchSet: 12 Gerrit-Owner: Tianyi Wang <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 02 Oct 2017 22:20:44 +0000 Gerrit-HasComments: Yes
