Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-3210: last/first_value() support for IGNORE NULLS
......................................................................


Patch Set 12:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3328/11/fe/src/main/java/com/cloudera/impala/analysis/AnalyticExpr.java
File fe/src/main/java/com/cloudera/impala/analysis/AnalyticExpr.java:

Line 593:    *    Set the end boundary to CURRENT_ROW.
> the 'if the start boundary ...' is part of the condition; merge into preced
Done


http://gerrit.cloudera.org:8080/#/c/3328/11/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test:

Line 1595: last_value(int_col) over(partition by tinyint_col order by id desc 
nulls first,
> long line
Done


Line 1617: |  functions: last_value(bigint_col)
> shouldn't this one be coalesced with node 06, given that they now have the 
We currently only group analytic fns in the same node if their order by is 
identical (enforced in AnalyticPlanner.isCompatible).

I talked with Matt briefly about it, and while it should be fine in this case, 
there's some concern about how this might interact with RANGE windows and other 
cases, so it seems like rather than addressing this here, it might be better to 
file a JIRA about it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic27525e2237fb54318549d2674f1610884208e9b
Gerrit-PatchSet: 12
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-HasComments: Yes

Reply via email to