Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE ......................................................................
Patch Set 4: > > > > (1 comment) > > > > > > The tests in this change use only 3 query options. Adding a new > > > section to the .test files to support only these 3 options > > wouldn't > > > be too much work. > > > > > > On the other hand, adding support for all the query options > that > > > Impala supports would be a lot harder. Probably we would have > to > > > implement that using some Java reflection trickery. > > > > I don't think you need to use Java reflection. The generated Java > > class for TQueryOptions has a number of helper functions to > search > > and set a field by name. So, for instance the QUERY_OPTIONS > section > > could have key=value pairs that correspond to query options. Then > > we could write a small function that parses the key value pairs > and > > uses the helper functions to check for valid query options and > set > > the values. Do you want to give it a try? Thanks > > Thanks. I'm still confused though how to implement setting enum > query options without reflection. e.g.: > ---- QUERYOPTIONS > COMPRESSION_CODEC=GZIP > > Here we have to know that the type of TQueryOptions.compression_codec > is org.apache.impala.thrift.THdfsCompression, otherwise we cannot > parse "GZIP". Am I missing something? If you detect that this query option refers to a compression_codec, the only thing you need to do is map the string literal "GZIP" to an instance of HdfsCompression (the latter is just an enum). From that you can get the THdfsCompression object that you pass in the setFieldValue() method. You don't need to handle all the query options in one go. You can add the infrastructure needed, i.e. parsing the key=value pairs, validating the key part and then simply add the logic to handle the options we're interesting in for this patch. Once the infrastructure is in place, then adding support for other options should be quite easy, but you don't need to do everything. -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-HasComments: No
