Hi Ashutosh, Long time no see!
On Thu, Feb 13, 2020 at 1:17 AM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > In postgresGetForeignJoinPaths(), I see > > /* Estimate costs for bare join relation */ > estimate_path_cost_size(root, joinrel, NIL, NIL, NULL, > &rows, &width, &startup_cost, &total_cost); > /* Now update this information in the joinrel */ > joinrel->rows = rows; > joinrel->reltarget->width = width; > > This code is good as well as bad. > > For a join relation, we estimate the number of rows in > set_joinrel_size_estimates() inside build_*_join_rel() and set the width of > the join when building the targetlist. For foreign join, the size estimates > may not be correct but width estimate should be. So updating the number of > rows looks good since it would be better than what > set_joinrel_size_etimates() might come up with but here are the problems with > this code > 1. The rows estimated by estimate_path_cost_size() are better only when > use_remote_estimates is true. So, we should be doing this only when > use_remote_estimate is true. I think it's actually harmless to do that even when use_remote_estimate=false because in that case we get the rows estimate from joinrel->rows in estimate_path_cost_size() and return to the caller the estimate as-is, IIRC. > 2. This function gets called after local paths for the first pair for this > join have been added. So those paths are not being judged fairly and perhaps > we might be throwing away better paths just because the local estimates with > which they were created were very different from the remote estimates. Yeah, but I'm not sure we really need to fix that because I think the remote-join path would usually win against any of local-join paths. Could you show me an example causing an issue? Best regards, Etsuro Fujita
