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

"Fragments separation represent the data exchange between Presto nodes." #23675

Closed
elharo opened this issue Sep 18, 2024 · 6 comments · Fixed by #23804
Closed

"Fragments separation represent the data exchange between Presto nodes." #23675

elharo opened this issue Sep 18, 2024 · 6 comments · Fixed by #23804

Comments

@elharo
Copy link
Contributor

elharo commented Sep 18, 2024

I'm not quite sure what this means or if it's correct, but at the least it has bad subject verb agreement. Maybe "Fragments' separations represent the data exchange between Presto nodes." but I'm not sure.

https://prestodb.io/docs/current/sql/explain.html

@aaneja
Copy link
Contributor

aaneja commented Sep 23, 2024

Original text -

Each plan fragment is executed by a single or multiple Presto nodes. Fragments separation represent the data exchange between Presto nodes

How about we replace it with 'Each plan fragment is executed by a single or multiple Presto nodes and represents a data exchange boundary between nodes'

cc: @steveburnett

@steveburnett
Copy link
Contributor

I don't think combining the sentences helps, honestly, so I'd like to leave "Each plan fragment is executed by a single or multiple Presto nodes." as it is because that seems simple to understand.

Addressing the second sentence and @elharo's concern, while the expressed subject verb agreement concern could be addressed by something like
"Fragment separation represents the data exchange between Presto nodes."
However, when I read this sentence I am not sure what is meant by "represent", and that may not be the best word here. Could someone who knows this topic suggest a revision of this sentence?

@tdcmeehan
Copy link
Contributor

How about we enrich the Presto concepts page and include sections for query plan and plan fragment. And just link the first sentence to the newly added plan fragment section, and remove this second sentence?

@tdcmeehan
Copy link
Contributor

tdcmeehan commented Sep 23, 2024

I took a quick stab at some initial documentation for Presto concepts, appreciate feedback:

Query Plan

A query plan is a sequence of steps used to access and manipulate data according to the SQL query. It is represented as a tree of nodes, with each node representing an operator. Because query plans can have different performance behavior, Presto uses a query optimizer to make query plans more efficient.

There are two phases of optimization: logical and physical. The logical phase of optimization transforms plans by only considering algorithmic complexity. The logically optimized query plan is then converted into a physical query plan, which is optimized for distributed execution and includes details such as the number and types of Presto servers which should process a query plan node, and how data is exchanged between them.

Plan Fragment

A plan fragment is a section of the physical query plan executed by tasks on different Presto servers.

@steveburnett
Copy link
Contributor

Thanks! Two minor revisions for shortness, with the intent to improve readability.

  • Maybe change
    "Because multiple query plans can return data in an equivalent way, but with different implications for performance, Presto uses a query optimizer to make query plans more efficient."
    to
    "Because query plans can have different performance behavior, Presto uses a query optimizer to make query plans more efficient."

  • Consider changing
    "After a plan has been logically optimized, the plan is then converted into a more specific form called the physical plan,"
    to
    "The logically optimized query plan is then converted into a physical query plan,"

@tdcmeehan
Copy link
Contributor

Thanks @steveburnett, updated my response to include your suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants