Sahil Takiar has posted comments on this change.

Change subject: IMPALA-4232: qgen: Hive does not support aggregates inside 
specific analytic clauses
......................................................................


Patch Set 5:

(5 comments)

@Taras, updated. Comments addressed

http://gerrit.cloudera.org:8080/#/c/4581/2//COMMIT_MSG
Commit Message:

Line 20: 
> It's still not exactly clear to me what is not allowed. Can you give an exa
Done


http://gerrit.cloudera.org:8080/#/c/4581/2/tests/comparison/query_generator.py
File tests/comparison/query_generator.py:

PS2, Line 584: basic
> It's not clear what the funcs argument means. Can you rename it something m
Done


Line 797:       # Check if the func_tree contains any analytic functions 
returned by
> this line and the comment above it should be right above line 805
Done


PS2, Line 803: # Plac
> this should be 4 spaces
Done


http://gerrit.cloudera.org:8080/#/c/4581/2/tests/comparison/query_profile.py
File tests/comparison/query_profile.py:

PS2, Line 693: get_analytic_funcs_that_cannot_co
> how about renaming this to allow_analytics_with_aggs?
I renamed it to get_analytic_funcs_that_cannot_contain_aggs() which is a little 
more verbose, but hopefully is more descriptive.

Changing it to allow_analytics_with_aggs would require returning the opposite 
set of functions (analytic functions that can contain aggs, rather than 
analytic functions that cannot contain aggs)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1096c4cde7ea52a52b39e31cd93242da53b549f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sahil Takiar <stak...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to