Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-3864: qgen: reduce likelihood of create_query() 
exceptions
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3720/3/tests/comparison/query_generator.py
File tests/comparison/query_generator.py:

Line 1468: if __name__ == '__main__':
How about putting everything in below this line into a function called 
something like generate_queries_for_manual_inspection() and calling that 
function. I think it will make the code more clear.


Line 1488:         col_idx = i * len(data_types) + type_idx
Maybe it would be more readable (and simpler) if we declared col_idx = 0 
outside and just incremented it here?


http://gerrit.cloudera.org:8080/#/c/3720/3/tests/comparison/query_profile.py
File tests/comparison/query_profile.py:

PS3, Line 90: And
does it make sense to have AND and OR here? It's already in conjunct_dijuncts


Line 518:     else:
How about putting this first? So change the if statement to if func_weights:


-- 
To view, visit http://gerrit.cloudera.org:8080/3720
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd9434a92973176aefb99e11e039209cac3cea65
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Michael Brown <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Michael Brown <[email protected]>
Gerrit-Reviewer: Taras Bobrovytsky <[email protected]>
Gerrit-HasComments: Yes

Reply via email to