Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9777 )
Change subject: IMPALA-4025: Part 1: Add percentile_disc aggregation function ...................................................................... Patch Set 1: (2 comments) initial comment... still reading. http://gerrit.cloudera.org:8080/#/c/9777/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9777/1//COMMIT_MSG@17 PS1, Line 17: select min(case when _percentile_row_number_diff_0 >= 0 then : _percentile_view.col end) from (select row_number() over (order by col) : - 0.5 * count(col) over() _percentile_row_number_diff_0, * from tbl) : _percentile_view pls format this so its easier to read. for example, the ..._diff_0, * from tbl) _percentile_view is difficult to follow/looks like an error. http://gerrit.cloudera.org:8080/#/c/9777/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java: http://gerrit.cloudera.org:8080/#/c/9777/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@363 PS1, Line 363: requiresPercentileRewrite() first thing that got my attention is the way the rewrite for this case is handled. it seems fairly special: percentile is one specific type of aggregate function yet it has its unique handling here. I don't have a better suggestion right now-- looking into this more-- but can you comment on the other ways you might have tried this and why they didn't work (e.g., more generic rewrite rule approach) -- To view, visit http://gerrit.cloudera.org:8080/9777 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacef7b3fcd74c4c73d88400ce27307c3baa0121e Gerrit-Change-Number: 9777 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Wed, 28 Mar 2018 16:18:28 +0000 Gerrit-HasComments: Yes
