Tianyi Wang 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 13: (3 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: y that the given query option has the : // 'expec > I don't understand that. Maybe this should say: Done http://gerrit.cloudera.org:8080/#/c/7805/12/be/src/service/query-options-test.cc@86 PS12, Line 86: // Create a shortcut function to verify that setting the 'option_def' option to 'str' > I think that should be reworded as per above. Done http://gerrit.cloudera.org:8080/#/c/7805/11/testdata/workloads/functional-query/queries/QueryTest/set.test File testdata/workloads/functional-query/queries/QueryTest/set.test: http://gerrit.cloudera.org:8080/#/c/7805/11/testdata/workloads/functional-query/queries/QueryTest/set.test@a145 PS11, Line 145: > Maybe we should leave these then. I forget, are these generic messages pro Tests involving error messages are restored. I don't understand what "generic messages produced by the query option code itself" means. Most error messages are from individual code for each individual option in a switch-case structure. Byte size options reuse error message from ParseMemValue() function. -- 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: 13 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:53:30 +0000 Gerrit-HasComments: Yes
