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 13: Code-Review+1 (1 comment) Like I said, let's avoid this much indirection/metaprogramming in general, but for this unit test I'm okay with it. Alex, can you also take a look to see if this satisfies what you wanted IMPALA-5425 to cover? (if no, we could go forward with the change as is and do another change, if that makes sense). 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: > Tests involving error messages are restored. I don't understand what "gener yeah, that's what I was asking -- whether checking one of each class of option types is sufficient or if each needs to be checked. sounds like for bytesize, one option is pretty good, but for some other ones, there is custom logic. -- 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: Tue, 03 Oct 2017 17:21:22 +0000 Gerrit-HasComments: Yes
