On Wed, Nov 13, 2019 at 10:04:36AM -0800, Mark Dilger wrote:
>
>
>On 11/13/19 7:28 AM, Tomas Vondra wrote:
>>Hi,
>>
>>here's an updated patch, with some minor tweaks based on the review and
>>added tests (I ended up reworking those a bit, to make them more like
>>the existing ones).
>
>Thanks, Tomas, for the new patch set!
>
>Attached are my review comments so far, in the form of a patch applied
>on top of yours.
>
Thanks.
1) It's not clear to me why adding 'const' to the List parameters would
be useful? Can you explain?
2) I think you're right we can change find_strongest_dependency to do
/* also skip weaker dependencies when attribute count matches */
if (strongest->nattributes == dependency->nattributes &&
strongest->degree >= dependency->degree)
continue;
That'll skip some additional dependencies, which seems OK.
3) It's not clear to me what you mean by
* TODO: Improve this code comment. Specifically, why would we
* ignore that no rows will match? It seems that such a discovery
* would allow us to return an estimate of 0 rows, and that would
* be useful.
added to dependencies_clauselist_selectivity. Are you saying we
should also compute selectivity estimates for individual clauses and
use Min() as a limit? Maybe, but that seems unrelated to the patch.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services