impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sahil Takiar (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4232: qgen: Hive does not support aggregates inside specific analytic clauses
Date Sat, 01 Oct 2016 18:00:46 GMT
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 <stakiar@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <stakiar@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message