I came across a couple of places in the planner that are checking for nonempty havingQual; but since these bits run after const-simplification of the HAVING clause, that produces the wrong answer for a constant-true HAVING clause (which'll be folded to empty). Correct code is to check root->hasHavingQual instead.
These mistakes only affect cost estimates, and they're sufficiently corner cases that it'd be hard even to devise a reliable test case showing a different plan choice. So I'm not very excited about this, and am thinking of committing only to HEAD. regards, tom lane
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index d98709e5e8..8d7500abfb 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -3357,7 +3357,7 @@ estimate_path_cost_size(PlannerInfo *root, * Get the retrieved_rows and rows estimates. If there are HAVING * quals, account for their selectivity. */ - if (root->parse->havingQual) + if (root->hasHavingQual) { /* Factor in the selectivity of the remotely-checked quals */ retrieved_rows = @@ -3405,7 +3405,7 @@ estimate_path_cost_size(PlannerInfo *root, run_cost += cpu_tuple_cost * numGroups; /* Account for the eval cost of HAVING quals, if any */ - if (root->parse->havingQual) + if (root->hasHavingQual) { QualCost remote_cost; diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 8fc28007f5..4ddaed31a4 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -2575,7 +2575,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel, if (parse->hasAggs || parse->groupClause || parse->groupingSets || - parse->havingQual || + root->hasHavingQual || parse->distinctClause || parse->sortClause || has_multiple_baserels(root))