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

docs(book): documentation for window rows vs range #3634

Merged
merged 4 commits into from
Oct 8, 2023
Merged

docs(book): documentation for window rows vs range #3634

merged 4 commits into from
Oct 8, 2023

Conversation

jangorecki
Copy link
Contributor

@jangorecki jangorecki commented Oct 8, 2023

Possibly it is overly verbose, but it is better like that, so there is no space for misinterpretation.

@eitsupi eitsupi changed the title documentation for window rows vs range docs(book): documentation for window rows vs range Oct 8, 2023
@eitsupi
Copy link
Member

eitsupi commented Oct 8, 2023

Thanks for the contribution!
In my opinion, this new code block is evaluable, so I removed the no-eval tag.
And I ran tests and added the snapshot.

@jangorecki
Copy link
Contributor Author

Yes code block is evaluable. I decided to not eval it because generated SQL doesn't really gives any more clarity over rows-vs-range, and due the to amount of lines it could possibly add more confusion, but I am fine either way.

@eitsupi
Copy link
Member

eitsupi commented Oct 8, 2023

Yes code block is evaluable. I decided to not eval it because generated SQL doesn't really gives any more clarity over rows-vs-range, and due the to amount of lines it could possibly add more confusion, but I am fine either way.

Yes, I understand the intent.
Perhaps the ideal here would be to be able to generate tables from prql code blocks using sqlite (or duckdb, I think the easiest way is to paste the Markdown output of duckdb CLI), but I think that would be a lot of work to try to implement in Rust.
(Of course, with knitr and prqlr this can be accomplished quickly, but that's the domain of https://github.com/eitsupi/querying-with-prql)

@jangorecki
Copy link
Contributor Author

Ideally would be to have a spoiler button that shows SQL if desired. Anyway, I just pasted output from playground and formatted the same way as other tables are formatted in the book.

Copy link
Member

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is excellent! Thank you @jangorecki

FWIW I agree with @eitsupi that having the SQL there is useful. While it's a bit noisy in this case, since most of the documentation is about the behavior rather the translation, it limits how confused someone can get :)

@max-sixty
Copy link
Member

Re having the results of queries in the book — #1895

@max-sixty max-sixty merged commit f2cd306 into PRQL:main Oct 8, 2023
33 checks passed
prql-bot pushed a commit that referenced this pull request Oct 8, 2023
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: eitsupi <ts1s1andn@gmail.com>
(cherry picked from commit f2cd306)
prql-bot added a commit that referenced this pull request Oct 8, 2023
Co-authored-by: Jan Gorecki <J.Gorecki@wit.edu.pl>
@snth
Copy link
Member

snth commented Oct 8, 2023

This is great! Thank you @jangorecki . ROWS vs RANGE is one of the most confusing things about window functions and SQL in general. When I try to recall my experience learning of learning this in SQL and the RDBMs docs I read at the time, they didn't make it that much clearer whereas this is an excellent example that explains it clearly! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants