Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document Window functions #6338

Closed
Tracked by #3058
alamb opened this issue May 11, 2023 · 3 comments · Fixed by #6402
Closed
Tracked by #3058

Document Window functions #6338

alamb opened this issue May 11, 2023 · 3 comments · Fixed by #6402
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented May 11, 2023

Is your feature request related to a problem or challenge?

@mustafasrepo and others have contributed to a sophisticated window function implementation in DataFusion like

select x, lag(x, 1) OVER (partition by y, order by x) FROM foo

However, the user guide does not appear to mention this

Describe the solution you'd like

  1. Note in the user guide that DataFusion supports window functions
  2. Add a new page for WindowFunctions (similar to the aggregate function list)

Note the docs (and instructions) are in https://github.com/apache/arrow-datafusion/tree/main/docs

The list of window functions is

  1. aggregate functions
  2. The list here https://docs.rs/datafusion-expr/24.0.0/datafusion_expr/window_function/enum.BuiltInWindowFunction.html

Describe alternatives you've considered

No response

Additional context

No response

@alamb alamb added documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers labels May 11, 2023
@toppyy
Copy link
Contributor

toppyy commented May 20, 2023

started work on this!

@toppyy
Copy link
Contributor

toppyy commented May 21, 2023

Made a draft PR.

The issue states that the PR should "Note in the user guide that DataFusion supports window functions". Any ideas where exactly to mention this? I had a look and I didn't find a place that felt suitable.

Also, I was on the fence about whether to describe the over-clause with each function as it seems repetative. The alternative would be to described the syntax for window functions on top of the page once in a general form (ie. something like function(expr) over([partition by expr2] [order by expr3 [ asc | desc ][, …]])). Any thoughts?

@alamb
Copy link
Contributor Author

alamb commented May 22, 2023

Thanks @toppyy !

The issue states that the PR should "Note in the user guide that DataFusion supports window functions". Any ideas where exactly to mention this? I had a look and I didn't find a place that felt suitable.

Perhaps we can add a new Window Functions page, following the model of Aggregate Functions? (UPDATE: I see this is exactly what you did in #6402 🤦 sorry for not checking that first)

Screenshot 2023-05-22 at 4 42 29 PM

Also, I was on the fence about whether to describe the over-clause with each function as it seems repetative. The alternative would be to described the syntax for window functions on top of the page once in a general form (ie. something like function(expr) over([partition by expr2] [order by expr3 [ asc | desc ][, …]])). Any thoughts?

I think describing the OVER clause once in general rather than for each function makes much more sense.

Perhaps we can take inspiration (and/or some of the text) from the postgres description: https://www.postgresql.org/docs/current/tutorial-window.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants