Skip to content

CBO documentation#127

Merged
findepi merged 2 commits intotrinodb:masterfrom
starburstdata:epic/cbo/pr/cbo-docs
Mar 13, 2019
Merged

CBO documentation#127
findepi merged 2 commits intotrinodb:masterfrom
starburstdata:epic/cbo/pr/cbo-docs

Conversation

@findepi
Copy link
Member

@findepi findepi commented Feb 1, 2019

@cla-bot
Copy link

cla-bot bot commented Feb 1, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@prestosql.io. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

@findepi findepi force-pushed the epic/cbo/pr/cbo-docs branch from 1e3e043 to 565f7f0 Compare February 1, 2019 10:32
@cla-bot
Copy link

cla-bot bot commented Feb 1, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@prestosql.io. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

@findepi findepi requested a review from electrum February 1, 2019 10:33
@findepi findepi force-pushed the epic/cbo/pr/cbo-docs branch from 565f7f0 to 7e28f25 Compare February 1, 2019 11:29
@cla-bot
Copy link

cla-bot bot commented Feb 1, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@prestosql.io. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

@findepi
Copy link
Member Author

findepi commented Feb 1, 2019

CLA check is red, but the commits are annotated with Extracted-From:. @electrum ready for review.

@cla-bot
Copy link

cla-bot bot commented Feb 12, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@prestosql.io. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

Please squash the fixups in the last commit into the previous commits as appropriate

Copy link
Member

Choose a reason for hiding this comment

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

Use dashes in file names (like other files)

Copy link
Member

Choose a reason for hiding this comment

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

Maybe put this after admin?

Copy link
Member

Choose a reason for hiding this comment

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

Please wrap documentation text at 80-100 characters. These line that wrap in GitHub are too long. Pre-formatted text like explain plans are fine to be as along as necessary, of course.

Copy link
Member

Choose a reason for hiding this comment

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

What is "root" here? Maybe just remove that word.

Copy link
Member

Choose a reason for hiding this comment

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

Use a link

:doc:`/sql/explain`

Copy link
Member

Choose a reason for hiding this comment

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

t does not exist in the example. How about

will show statistics for the table layout representing a subset of data after applying the given filtering condition

Copy link
Member

Choose a reason for hiding this comment

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

Change since we don't support a column list in the select clause

The filtering condition used in the ``WHERE`` clause can reference table columns.

Although I'm not sure this sentence adds any value. What else would it reference or why else would it be used?

Copy link
Member

Choose a reason for hiding this comment

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

How about

If the ``WHERE`` clause filters out all of the data in the table, then
``SHOW STATS`` will return no statistics, as shown in the example below.

Copy link
Member

Choose a reason for hiding this comment

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

Remove this section since it is no longer supported

Copy link
Member

Choose a reason for hiding this comment

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

Please update this section to use Presto's ANALYZE

Copy link
Member Author

Choose a reason for hiding this comment

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

I am dropping this whole section (below for reference). I added For the Hive connector, refer to :ref:`Hive connector <hive_analyze>` documentation if you want to update table statistics. instead.

Updating Statistics For Hive Tables
-----------------------------------

For the Hive connector, Presto uses the statistics that are managed by Hive and
exposed via the Hive metastore API.  Depending on the Hive configuration, table
statistics may not be updated automatically.

If statistics are not updated automatically, the user needs to trigger a
statistics update via the Hive CLI.

The following command can be used in the Hive CLI to update table statistics for
non-partitioned table ``t``::

        hive> ANALYZE TABLE t COMPUTE STATISTICS;
        hive> ANALYZE TABLE t COMPUTE STATISTICS FOR COLUMNS;

For partitioned tables, partitioning information must be specified in the
command.  Assuming table ``t`` has two partitioning keys ``a`` and ``b``, the
following command would update the table statistics for all partitions::

        hive> ANALYZE TABLE t PARTITION (a, b) COMPUTE STATISTICS FOR COLUMNS;

It is also possible to update statistics for just a subset of partitions.  This
command will update statistics for all partitions for which partitioning key
``a`` is equal to ``1``::

        hive> ANALYZE TABLE t PARTITION (a=1, b) COMPUTE STATISTICS FOR COLUMNS;

And this command will update statistics for just one partition::

        hive> ANALYZE TABLE t PARTITION (a=1, b=5) COMPUTE STATISTICS FOR COLUMNS;

For documentation on Hive's statistics mechanism see
https://cwiki.apache.org/confluence/display/Hive/StatsDev

Copy link
Member Author

Choose a reason for hiding this comment

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

@findepi findepi force-pushed the epic/cbo/pr/cbo-docs branch 2 times, most recently from bb09b12 to 0d33efe Compare March 6, 2019 12:21
@trinodb trinodb deleted a comment from cla-bot bot Mar 6, 2019
@trinodb trinodb deleted a comment from cla-bot bot Mar 6, 2019
@findepi
Copy link
Member Author

findepi commented Mar 6, 2019

ac

Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

A few nits, otherwise looks great!

Copy link
Member

Choose a reason for hiding this comment

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

Nit: don't wrap a single word. We wrap to improve readability, but this is less readable

Copy link
Member

Choose a reason for hiding this comment

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

Add "the" at end

refer to the ...

Copy link
Member

Choose a reason for hiding this comment

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

How about

refer to the ... documentation to learn how to update table statistics.

Copy link
Member

Choose a reason for hiding this comment

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

Comma after "available"

Copy link
Member

Choose a reason for hiding this comment

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

The sentence sounds tautological. How about

With cost based join distribution selection, Presto automatically chooses to use a partitioned or broadcast join.

Copy link
Member

@electrum electrum Mar 8, 2019

Choose a reason for hiding this comment

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

Remove code quotes and lowercase on the description part, since we are describing the type of join, not referencing a config option

 * ``BROADCAST`` - broadcast join distribution is used for all joins

Copy link
Member

Choose a reason for hiding this comment

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

Let's change this to

* Partitioned: each node participating in the query builds a hash table
  from only fraction of the data
* Broadcast: each node participating in the query builds a hash table
  from all of the data (data is replicated to each node)

Since we are talking about logical operations, not config options. Also a few minor wording changes.

@cla-bot
Copy link

cla-bot bot commented Mar 12, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@prestosql.io. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

@findepi findepi assigned electrum and unassigned findepi Mar 12, 2019
@findepi
Copy link
Member Author

findepi commented Mar 12, 2019

ac

@findepi findepi force-pushed the epic/cbo/pr/cbo-docs branch from 2d98e04 to 6d48e0f Compare March 13, 2019 08:48
@cla-bot
Copy link

cla-bot bot commented Mar 13, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@prestosql.io. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

@findepi findepi merged commit f79058e into trinodb:master Mar 13, 2019
@findepi findepi deleted the epic/cbo/pr/cbo-docs branch March 13, 2019 10:23
aweisberg added a commit to aweisberg/presto that referenced this pull request Jan 7, 2020
Extracted-From: https://github.com/starburstdata/presto

Cherry-pick of trinodb/trino#127

Co-authored-by: Ariel Weisberg <aweisberg@fb.com>
aweisberg added a commit to aweisberg/presto that referenced this pull request Jan 7, 2020
Extracted-From: https://github.com/starburstdata/presto

Cherry-pick of trinodb/trino#127

Co-authored-by: Łukasz Osipiuk <lukasz@osipiuk.net>
rschlussel pushed a commit to prestodb/presto that referenced this pull request Jan 7, 2020
Extracted-From: https://github.com/starburstdata/presto

Cherry-pick of trinodb/trino#127

Co-authored-by: Ariel Weisberg <aweisberg@fb.com>
rschlussel pushed a commit to prestodb/presto that referenced this pull request Jan 7, 2020
Extracted-From: https://github.com/starburstdata/presto

Cherry-pick of trinodb/trino#127

Co-authored-by: Łukasz Osipiuk <lukasz@osipiuk.net>
kaikalur pushed a commit to kaikalur/presto that referenced this pull request Jan 22, 2020
Extracted-From: https://github.com/starburstdata/presto

Cherry-pick of trinodb/trino#127

Co-authored-by: Ariel Weisberg <aweisberg@fb.com>
kaikalur pushed a commit to kaikalur/presto that referenced this pull request Jan 22, 2020
Extracted-From: https://github.com/starburstdata/presto

Cherry-pick of trinodb/trino#127

Co-authored-by: Łukasz Osipiuk <lukasz@osipiuk.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants