Skip to content
This repository has been archived by the owner on Sep 27, 2019. It is now read-only.

Add cardinality estimate to AbstractPlan object #1475

Merged
merged 11 commits into from
Sep 18, 2018

Conversation

GustavoAngulo
Copy link
Member

@GustavoAngulo GustavoAngulo commented Aug 31, 2018

This PR adds the optimizer's cardinality estimate to the AbstractPlan object for use during plan execution. Includes test cases to verify it works. This is part of #1467 to add more optimizer estimates to plans.

Additionally, this PR adds an optimizer_test_util class with useful helper functions for creating test tables and generating plans. Future and possibly existing test cases should inherit from this class.

@coveralls
Copy link

coveralls commented Aug 31, 2018

Coverage Status

Coverage increased (+0.2%) to 76.736% when pulling a8c779c on GustavoAngulo:add_cardinality_estimate into e738acb on cmu-db:master.

Copy link
Member

@linmagit linmagit left a comment

Choose a reason for hiding this comment

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

In general looks good. Some minor comments.

predicate_stats, op->predicates);
// Use predicates to estimate cardinality
if (table_stats->GetColumnCount() == 0) {
root_group->SetNumRows(0);
Copy link
Member

Choose a reason for hiding this comment

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

When will this happen? When the table is empty? Or no stats exists? Or the cardinality estimation is 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

This would happen if ColumnStatsCatalog::GetTableStats returned no column stats. I believe this would happen if the catalog did not have stats for the table. This is assuming its not possible to have a table without columns.

src/optimizer/plan_generator.cpp Show resolved Hide resolved
test/optimizer/cardinality_test.cpp Outdated Show resolved Hide resolved
@apavlo apavlo merged commit 1fc8b55 into cmu-db:master Sep 18, 2018
mtunique pushed a commit to mtunique/peloton that referenced this pull request Apr 16, 2019
* Clarified variable name

* Added cardinality and test

* Add optimizer testing class

* Remove debugging code

* Revert default estimate to fix broken test

* Formatting

* Removed unecessary override in Cardinality test

* More comments and clean up tests
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants