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

Manage Postgres parallel plans when estimating row count #37619

Merged
merged 1 commit into from Jul 20, 2020

Conversation

troopa81
Copy link
Contributor

@troopa81 troopa81 commented Jul 6, 2020

Description

Fixes #37342, follows #32114

Previous PR wasn't dealing with possible parallel plan when estimating count. This one use a simpler SQL request where the number of rows is contained in the top node whether it is parallel (sum up the children gather node count) or not

@github-actions github-actions bot added this to the 3.16.0 milestone Jul 6, 2020
@troopa81 troopa81 force-pushed the fix_estimatecount_parallel branch from 7a665d1 to ca9dd37 Compare July 6, 2020 09:29
@troopa81 troopa81 closed this Jul 6, 2020
@troopa81 troopa81 reopened this Jul 6, 2020
@saberraz
Copy link
Contributor

Does this need to be backported?

@saberraz
Copy link
Contributor

@wonder-sk would it be possible to review this PR?

@troopa81
Copy link
Contributor Author

Does this need to be backported?

for 3.14 yes, for 3.10 I believe that it was proposed to wait & see if everything is fine in 3.14 before backporting.

@wonder-sk
Copy link
Member

Hmm... what do you think of using this method (parsing execution plan from JSON) only with views - and for tables use the old method of reading the estimated counts directly from pg_class? The reliance on execution plan and its JSON is a bit scary to me and I would expect it can break when there are changes to the planner or to the output format...

@troopa81
Copy link
Contributor Author

Hmm... what do you think of using this method (parsing execution plan from JSON) only with views - and for tables use the old method of reading the estimated counts directly from pg_class?

It would cost an extra request to check if mQuery is a view or a table (calling relkind)

The reliance on execution plan and its JSON is a bit scary to me and I would expect it can break when there are changes to the planner or to the output format...

I understand your concerns but the proposed version relies on the number of rows of plan top node, so the planner could change but you'll always get an estimated row count in top node. As for the format, it could indeed change but as it's kind of a API breaking change, I would say it shouldn't happen often (never?). And if it happens we would have to fix the view part anyway.

@wonder-sk
Copy link
Member

Okay - let's leave it like this and hope PostgreSQL won't change the syntax in the future... or make the texts translatable :-)

@troopa81
Copy link
Contributor Author

@wonder-sk Would you mind merging it?

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

Successfully merging this pull request may close these issues.

[postgres] Wrong feature counts when using estimated metadata
3 participants