Matthew Jacobs has posted comments on this change. Change subject: IMPALA-1882: Remove ORDER BY restriction from first_value()/last_value() ......................................................................
Patch Set 2: (5 comments) looks good! just a few small nits then I'll submit it http://gerrit.cloudera.org:8080/#/c/7502/2/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java File fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java: PS2, Line 231: String fnName = fn.functionName(); : return fnName.equals(LAST_VALUE) || fnName.equals(FIRST_VALUE) : || fnName.equals(LAST_VALUE_IGNORE_NULLS) : || fnName.equals(FIRST_VALUE_IGNORE_NULLS); : } Please use isAnalyticFn() as the above methods do. http://gerrit.cloudera.org:8080/#/c/7502/2/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test File testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test: PS2, Line 2017: first_val remove this alias, for simplicity PS2, Line 2037: first_val same PS2, Line 2057: first_val remove; this alias is just confusing given this is the last_value fn :) PS2, Line 2077: first_val same -- To view, visit http://gerrit.cloudera.org:8080/7502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5a3a56833ac062839629353ea240b361bc727d96 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
