Add documentation for empty window frames

[aditi-pandit]

[netlify[bot]]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	f7a66e4bf2322023159d92c91f2ac7ed55dcc94f
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/669dfd6561a00b00080ef942

[aditi-pandit]

@aditi-pandit Aditi, thank you for working on improving documentation for window functions. It feels like this new doc contains a mix of user-facing documentation that clarifies the semantics of empty and partial frames and some implementation specific details of the Window operator. I wonder if it makes sense to move user-facing details to https://facebookincubator.github.io/velox/functions/presto/window.html and implementation details to the comments in the code.

@mbasmanova : https://facebookincubator.github.io/velox/functions/presto/window.html and also https://facebookincubator.github.io/velox/functions/spark/window.html in turn are mainly for the functions. I feel adding a link from the functions doc to the window frames doc is better. wdyt ?

I wrote this doc somewhat in line with join and anti-join docs since they too provide many examples with some sense of the code. Since empty frames influence the WindowFunction API, I felt it should be mentioned. That part is more developer facing than user-facing. But since Velox users are mostly developers the boundaries are vague.

[aditi-pandit]

@mbasmanova : Thanks for the review. Have addressed your comments.

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the PR, make sure you've addressed reviewer comments, and rebase on the latest main. Thank you for your contributions!

[aditi-pandit]

@mbasmanova : We now have a window function developer guide at https://facebookincubator.github.io/velox/develop/window.html.

This PR enhances the docs for the usage of valid_frames argument in it.

Please can you review.

[aditi-pandit]

Hi @aditi-pandit, thank you for adding the documentation! I left a few comments.

For the figure, could you add the index of the current row for each of the example frame so make it more clear that they're concrete frames during window sliding?

Thanks @kagamiori for your review.

Have addressed your review comments. PTAL.

[aditi-pandit]

Hi @aditi-pandit, thank you for iterating on this PR. It looks good to me except a few nits.

Thanks @kagamiori. Have addressed the review comments.PTAL.

[facebook-github-bot]

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@kagamiori merged this pull request in facebookincubator/velox@e25fba1cebfc6e3148037037d3406545abf06108.

[conbench-facebook[bot]]

Conbench analyzed the 1 benchmark run on commit e25fba1c.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.