impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3210: last/first value() support for IGNORE NULLS
Date Wed, 13 Jul 2016 19:01:51 GMT
Thomas Tauber-Marshall has posted comments on this change.

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


Patch Set 10:

(9 comments)

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

Line 589:    * 4. Reverse the ordering and window, and flip first_value() ans last_value(),
if the
> spelling
Done


Line 593:    * 5. first_value():
> unify description of these cases: start with the condition, then describe t
Done


Line 594:    *    Set the upper boundary to CURRENT_ROW if the lower boundary is
> start/end instead of lower/upper
Done


Line 730:     if (window_ != null
> make this look more similar to the if stmt for case 3
Done


http://gerrit.cloudera.org:8080/#/c/3328/10/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeExprsTest.java:

Line 719:     AnalyzesOk("select first_value(tinyint_col ignore nulls) over (order by id)
from "
> also add last_value and also include the non-ignore variants
Done


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

Line 1526: # Set the upper boundary to CURRENT_ROW for first_value() if the lower boundary
is
> upper and lower are ambiguous (i always picture upper to mean end bound). b
Done


Line 1535: #                          range between 4 preceding and unbounded following),
> remove obsolete lines
Done


Line 1544: # window, order, and function shoud be revered
> spelling
Done


Line 1567: |  window: ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
> isn't this the same as 'range between ub preceding and current row'? in tha
Done


-- 
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: 10
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message