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


Reply via email to