Dmitry Lychagin has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/3448 )

Change subject: [NO ISSUE] Document window functions.
......................................................................


Patch Set 1:

(17 comments)

Looks good! Just a few comments.

https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/0_toc.md
File asterixdb/asterix-doc/src/main/markdown/builtins/0_toc.md:

https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/0_toc.md@38
PS1, Line 38: * [Window Clause (OVER Clause)](#WindowClause)
OVER clause is strictly speaking not a window clause.
There's a separate WINDOW clause defined by the SQL standard:
https://www.postgresql.org/docs/11/sql-select.html#SQL-WINDOW
We're currently not supporting the standard WINDOW clause, but we will in the 
future.


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/14_window.md
File asterixdb/asterix-doc/src/main/markdown/builtins/14_window.md:

https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/14_window.md@260
PS1, Line 260:       tied objects in the window frame, the function returns the 
lowest value
"lowest"? what if ORDER BY had DESC modifier?


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/14_window.md@491
PS1, Line 491:       tied objects in the window frame, the function returns the 
highest
"highest"? What if ORDER BY had DESC modifier?


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/14_window.md@691
PS1, Line 691:     * [Nth Val From](#nthval-from): (Optional) Determines where 
the function
trailing whitespace


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/14_window.md@743
PS1, Line 743:     * If the window frame is defined by `RANGE` or `GROUPS`, and 
there are
lowest/highest will also depend on the ORDER BY modifier (DESC/ASC)


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md
File asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md:

https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@20
PS1, Line 20: ## <a id="WindowClause">Window Clause (OVER Clause)</a> ##
May be instead of "window clause" we should call it "window function call 
expressions" or "window function calls". These are function call expressions 
with OVER clause. The OVER clause either provides window specification directly 
(which is what we support now), or in the future will be able to refer to a 
WINDOW clause that defined the window specification.


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@23
PS1, Line 23: the order of objects within the window, and the size of the 
window frame.
May be say "the order of tuples within those partitions". Also, I'd may be say 
"contents of the window frame", instead of just "size"


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@24
PS1, Line 24: The `OVER` keyword introduces the window clause.
" ... introduces the window function call" ?


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@94
PS1, Line 94: The AS keyword enables you to specify an alias for the window 
clause.
".. an alias for the window frame contents". It introduces a variable which 
will be bound to the contents of the frame.


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@97
PS1, Line 97: window clause using this alias, for example:
may be "refers to this alias"? The alias is just a variable that's introduced 
by the OVER clause that's only visible inside window function call arguments.


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@99
PS1, Line 99:     FROM source
I'd give an alias for "source" too: "FROM source AS src", then change "FROM 
alias SELECT VALUE alias.source.expr" to "FROM alias AS a SELECT VALUE 
a.src.field_name"


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@125
PS1, Line 125: ### <a id="window-clause">Window Clause</a> ###
Again, we probably should not call it "Window Clause" to avoid confusion with 
standard WINDOW clause that we'll support in the future.


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@137
PS1, Line 137: The **window partition clause** groups the query results into 
partitions using
"groups the query results" might be confusing. Unlike GROUP BY window function 
calls do not change the number of tuples in the result, so they don't introduce 
groups. May be say "divides"? Also    , "query results" is not strictly 
correct. Window function call operates an stream of binding tuples that are 
input to the clause where this window function call appears. This is consistent 
with how all top-level clauses operate. For more information look for "binding 
tuple" at 
https://ci.apache.org/projects/asterixdb/sqlpp/manual.html#Select_clauses


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@150
PS1, Line 150: The **window order clause** determines how objects are ordered 
within each
I think we call them "tuples" in this documentation, not "objects".


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@238
PS1, Line 238: The ordering term expression must evaluate to a number.
In AsterixDB it can also evaluate to date/time/datetime. See 
https://ci.apache.org/projects/asterixdb/datamodel.html
The "Expression" in PRECEDING/FOLLOWING must evaluate to duration. See 
https://github.com/apache/asterixdb/blob/master/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/window/pg_win/pg_win.50.query.sqlpp


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/15_over.md@252
PS1, Line 252: In JSON, dates and times are represented as a string in ISO-8601 
standard.
AsterixDB also has temporal data types (date, datetime, duration, etc). RANGE 
boundaries can be durations. See 
https://github.com/apache/asterixdb/blob/master/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/window/pg_win/pg_win.50.query.sqlpp

Let's discuss the best way to document this.


https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/9_aggregate_sql.md
File asterixdb/asterix-doc/src/main/markdown/builtins/9_aggregate_sql.md:

https://asterix-gerrit.ics.uci.edu/#/c/3448/1/asterixdb/asterix-doc/src/main/markdown/builtins/9_aggregate_sql.md@35
PS1, Line 35: Aggregate functions may also be used as window functions when 
they are used with a window clause,
I'd say "OVER clause" instead of "window clause". We need to keep in mind that 
we'll be supporting standard WINDOW clause in the future.



--
To view, visit https://asterix-gerrit.ics.uci.edu/3448
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52d6e97a27c2fa51208810c6ac3d98cb21a0e2b1
Gerrit-Change-Number: 3448
Gerrit-PatchSet: 1
Gerrit-Owner: Simon Dew <[email protected]>
Gerrit-Reviewer: Anon. E. Moose (1000171)
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Simon Dew <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Comment-Date: Mon, 17 Jun 2019 21:09:33 +0000
Gerrit-HasComments: Yes

Reply via email to