Tim Armstrong has posted comments on this change. Change subject: IMPALA-5425: Add test for validating input when setting query options ......................................................................
Patch Set 7: The really nice thing about the patch is that all the tests are all table-driven so adding query options should just require adding entries to tables describing the valid values of each option (this also helps enforce that behaviour of query options is consistent).I think we should preserve that aspect regardless. >From what I can see the use of templates is directly in service of supporting >table-driven tests without copying and pasting the same code for every type >(which would be particularly painful for the enums). I think there are some >opportunities to remove indirection at the cost of repetition. E.g. if we >think people will be confused by functions returning functions we could avoid >that, but it makes sense to me and avoids a lot of repetition in the test >cases themselves. -- To view, visit http://gerrit.cloudera.org:8080/7805 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I510e02bb0776673d8cbfc22b903831882c6908d7 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi Wang <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: No
