Taras Bobrovytsky has posted comments on this change. Change subject: Misc query generator improvements ......................................................................
Patch Set 2: (13 comments) http://gerrit.cloudera.org:8080/#/c/2445/2/tests/comparison/discrepancy_searcher.py File tests/comparison/discrepancy_searcher.py: Line 294: mem_limit=randint(1024 ** 3, 10 * 1024 ** 3), Maybe it's better to move all these constants into query_profile? Line 336: self.set_impala_query_optons(cursor) I think we should not be setting the options all the time. How about adding an option to the query_profile where there is a certain probability that query options will be set? Line 527: def search(self, number_of_test_queries): I'm curious, did you manage to find many front end bugs with this? http://gerrit.cloudera.org:8080/#/c/2445/2/tests/comparison/query_generator.py File tests/comparison/query_generator.py: Line 402 It looks like null_args are overwritten below so this line is pointless. Was this a bug? Line 395: if func.contains_subquery: how about: null_args = subquery_not_allowed_null_args if func.contains_subquery else ... Line 438: only_accepts how about accepts_only? Line 1193: assert possible_left_table_exprs Why do we need an assert here? What happens if it fails? Line 1237: None why not set this to 0? Line 1246: arg_stop_idx = len(func.args) arg_stop_ids = arg_stop_idx or len(func.args) Line 1250: if null_args is None: null_args = null_args or list() Line 1443: if not root_func: how about changing order? (to get rid of the not) if root_func: ... http://gerrit.cloudera.org:8080/#/c/2445/2/tests/comparison/query_profile.py File tests/comparison/query_profile.py: Line 436: and not any(a for a in s.args if a.is_subquery), how about: and not any(a.is_subquery for a in s.agrs) Line 492: in I prefer if signature.func not in func_weights -- To view, visit http://gerrit.cloudera.org:8080/2445 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I75a7e6a1d96e5d92368f73c1e5e6a6b288932497 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Casey Ching <[email protected]> Gerrit-Reviewer: Casey Ching <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
