Horton

Thread

Commits

  1. Reset API of clause_selectivity()

  2. Collect and use multi-column dependency stats

  3. Implement multivariate n-distinct coefficients

  1. pgsql: Collect and use multi-column dependency stats

    Simon Riggs <simon@2ndquadrant.com> — 2017-04-05T22:06:02Z

    Collect and use multi-column dependency stats

Follow on patch in the multi-variate statistics patch series.

CREATE STATISTICS s1 WITH (dependencies) ON (a, b) FROM t;
ANALYZE;
will collect dependency stats on (a, b) and then use the measured
dependency in subsequent query planning.

Commit 7b504eb282ca2f5104b5c00b4f05a3ef6bb1385b added
CREATE STATISTICS with n-distinct coefficients. These are now
specified using the mutually exclusive option WITH (ndistinct).

Author: Tomas Vondra, David Rowley
Reviewed-by: Kyotaro HORIGUCHI, Álvaro Herrera, Dean Rasheed, Robert Haas
and many other comments and contributions
Discussion: https://postgr.es/m/56f40b20-c464-fad2-ff39-06b668fac47c@2ndquadrant.com

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/2686ee1b7ccfb9214064d4d2a98ea77382880306

Modified Files
--------------
contrib/file_fdw/file_fdw.c                      |    1 +
contrib/postgres_fdw/postgres_fdw.c              |    5 +-
doc/src/sgml/catalogs.sgml                       |    9 +
doc/src/sgml/planstats.sgml                      |  154 +++
doc/src/sgml/ref/create_statistics.sgml          |   42 +-
src/backend/catalog/system_views.sql             |    3 +-
src/backend/commands/statscmds.c                 |   17 +-
src/backend/optimizer/path/clausesel.c           |  113 ++-
src/backend/optimizer/path/costsize.c            |   25 +-
src/backend/optimizer/util/orclauses.c           |    4 +-
src/backend/optimizer/util/plancat.c             |   12 +
src/backend/statistics/Makefile                  |    2 +-
src/backend/statistics/README                    |   68 +-
src/backend/statistics/README.dependencies       |  119 +++
src/backend/statistics/dependencies.c            | 1079 ++++++++++++++++++++++
src/backend/statistics/extended_stats.c          |  105 ++-
src/backend/utils/adt/ruleutils.c                |   54 +-
src/backend/utils/adt/selfuncs.c                 |   20 +-
src/bin/psql/describe.c                          |   12 +-
src/include/catalog/pg_cast.h                    |    4 +
src/include/catalog/pg_proc.h                    |    9 +
src/include/catalog/pg_statistic_ext.h           |    7 +-
src/include/catalog/pg_type.h                    |    4 +
src/include/optimizer/cost.h                     |    6 +-
src/include/statistics/extended_stats_internal.h |    5 +
src/include/statistics/statistics.h              |   44 +
src/test/regress/expected/opr_sanity.out         |    3 +-
src/test/regress/expected/rules.out              |    3 +-
src/test/regress/expected/stats_ext.out          |  110 ++-
src/test/regress/expected/type_sanity.out        |    7 +-
src/test/regress/sql/stats_ext.sql               |   68 ++
31 files changed, 2035 insertions(+), 79 deletions(-)

  2. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-04-05T22:48:57Z

    Simon Riggs <simon@2ndQuadrant.com> writes:
> Collect and use multi-column dependency stats

The buildfarm is unhappy about the fact that this changed the API
for clauselist_selectivity().  I am not convinced that that change
was a good idea, so before telling FDW authors that they need to
change their code, I'd like to hear a defense of the API change.
Why not just use the existing varRelid parameter for that?  Why
is there an assumption that only one rel's extended stats will
ever be of interest?  This function does get used for join clauses.

			regards, tom lane

  3. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    David Rowley <david.rowley@2ndquadrant.com> — 2017-04-05T23:20:36Z

    On 6 April 2017 at 10:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Simon Riggs <simon@2ndQuadrant.com> writes:
>> Collect and use multi-column dependency stats
>
> The buildfarm is unhappy about the fact that this changed the API
> for clauselist_selectivity().  I am not convinced that that change
> was a good idea, so before telling FDW authors that they need to
> change their code, I'd like to hear a defense of the API change.
> Why not just use the existing varRelid parameter for that?  Why
> is there an assumption that only one rel's extended stats will
> ever be of interest?  This function does get used for join clauses.

Because varReliId is often passed in as 0, and that meant we'd have to
write some code to check of the clause was made up of RestrictInfos
from a single relation or not, and look for extended stats on that
singleton rel. Given the number of times this function gets called
during planning, especially so with queries involving many joins, I
think it's best to not have to extract the correct relids each time. I
coded it that way to reduce the overhead during planning, which is
something that often comes up with planner patches.

FWIW, I found this function being called 72 times in a 5 way join
search problem.

Perhaps there is a nicer way to do this, I just couldn't think of it.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  4. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-04-05T23:33:03Z

    David Rowley <david.rowley@2ndquadrant.com> writes:
> On 6 April 2017 at 10:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The buildfarm is unhappy about the fact that this changed the API
>> for clauselist_selectivity().  I am not convinced that that change
>> was a good idea, so before telling FDW authors that they need to
>> change their code, I'd like to hear a defense of the API change.

> Because varReliId is often passed in as 0, and that meant we'd have to
> write some code to check of the clause was made up of RestrictInfos
> from a single relation or not, and look for extended stats on that
> singleton rel.

Generally, if it's passed as zero, that's a good clue that the clause
*is* a join clause.  In any case, this defense fails to address my
other question, which is what's going to happen to this API when you
want to use extended stats in join-clause estimates, which I'd expect
to surely happen before very long.

Also, I find it hard to believe that a bms_get_singleton_member call is
going to be material in comparison to all the work that will be invoked
indirectly via whatever selectivity estimation function gets called for
each clause.  Even a single catcache fetch would swamp that.

So no, you have not convinced me that this isn't a broken design.

> FWIW, I found this function being called 72 times in a 5 way join
> search problem.

And you measured the overhead of doing it the other way to be ... ?
Premature optimization and all that.

			regards, tom lane

  5. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    David Rowley <david.rowley@2ndquadrant.com> — 2017-04-06T01:05:24Z

    On 6 April 2017 at 11:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> David Rowley <david.rowley@2ndquadrant.com> writes:
>> On 6 April 2017 at 10:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> The buildfarm is unhappy about the fact that this changed the API
>>> for clauselist_selectivity().  I am not convinced that that change
>>> was a good idea, so before telling FDW authors that they need to
>>> change their code, I'd like to hear a defense of the API change.
>
>> Because varReliId is often passed in as 0, and that meant we'd have to
>> write some code to check of the clause was made up of RestrictInfos
>> from a single relation or not, and look for extended stats on that
>> singleton rel.
>
> Generally, if it's passed as zero, that's a good clue that the clause
> *is* a join clause.  In any case, this defense fails to address my
> other question, which is what's going to happen to this API when you
> want to use extended stats in join-clause estimates, which I'd expect
> to surely happen before very long.
>
> Also, I find it hard to believe that a bms_get_singleton_member call is
> going to be material in comparison to all the work that will be invoked
> indirectly via whatever selectivity estimation function gets called for
> each clause.  Even a single catcache fetch would swamp that.
>
> So no, you have not convinced me that this isn't a broken design.
>
>> FWIW, I found this function being called 72 times in a 5 way join
>> search problem.
>
> And you measured the overhead of doing it the other way to be ... ?
> Premature optimization and all that.

I tested with the attached, and it does not seem to hurt planner
performance executing:

explain select * from ab ab1
inner join ab ab2 on ab1.a = ab2.a and ab1.b = ab2.b
inner join ab ab3 on ab1.a = ab3.a and ab1.b = ab3.b
inner join ab ab4 on ab1.a = ab4.a and ab1.b = ab4.b
inner join ab ab5 on ab1.a = ab5.a and ab1.b = ab5.b
inner join ab ab6 on ab1.a = ab6.a and ab1.b = ab6.b
inner join ab ab7 on ab1.a = ab7.a and ab1.b = ab7.b
inner join ab ab8 on ab1.a = ab8.a and ab1.b = ab8.b;

after having executed:

create table ab (a int, b int);

I get:

find_relation_from_clauses
tps = 48.992918 (excluding connections establishing)
tps = 49.060407 (excluding connections establishing)
tps = 49.075815 (excluding connections establishing)

Master

tps = 48.938027 (excluding connections establishing)
tps = 48.066274 (excluding connections establishing)
tps = 48.727089 (excluding connections establishing)

running pgbench -n -T 60 -f 8wayjoin.sql


-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  6. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    David Rowley <david.rowley@2ndquadrant.com> — 2017-04-06T01:10:48Z

    On 6 April 2017 at 13:05, David Rowley <david.rowley@2ndquadrant.com> wrote:
> I tested with the attached, and it does not seem to hurt planner
> performance executing:

Here's it again, this time with a comment on the
find_relation_from_clauses() function.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  7. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2017-04-06T06:03:16Z

    At Thu, 6 Apr 2017 13:10:48 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f8Um=BvRmgcb3u6ze1q1xL7A1VKTVF9s2R1_UfRqx8q5w@mail.gmail.com>
> On 6 April 2017 at 13:05, David Rowley <david.rowley@2ndquadrant.com> wrote:
> > I tested with the attached, and it does not seem to hurt planner
> > performance executing:
> 
> Here's it again, this time with a comment on the
> find_relation_from_clauses() function.

It seems to work as the same as the previous version with
additional cost to scan over restrict clauses. But separate loop
over clauses is additional overhead in any cases even irrelavant
to functional dependency.  The more columns are in the relation,
the longer time bms_get_singleton_member takes. Although I'm not
sure how much it hurts performance and I can't think of a good
alternative right now, I think that the overhead should be
avoided anyhow.

At Thu, 6 Apr 2017 13:05:24 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f_gB=gyZn8wMw0v8uCKD1nYeWyNYCXKz=+Oo0yR4RRyiA@mail.gmail.com>
> > And you measured the overhead of doing it the other way to be ... ?
> > Premature optimization and all that.
> 
> I tested with the attached, and it does not seem to hurt planner
> performance executing:

Here, bms_singleton_member takes longer time if the relation has
many columns and there's a functional dependency covering the
columns at the very tail. Maybe only two are not practical for
testing.

Even if it doesn't impact performance detectably, if only one
attribute is needed, an AttrNumber member in context will be
sufficient. No bitmap operation seems required in
dependency_compatible_walker and it can bail out by the second
attribute.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  8. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    David Rowley <david.rowley@2ndquadrant.com> — 2017-04-06T06:59:35Z

    On 6 April 2017 at 18:03, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> At Thu, 6 Apr 2017 13:10:48 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f8Um=BvRmgcb3u6ze1q1xL7A1VKTVF9s2R1_UfRqx8q5w@mail.gmail.com>
>> On 6 April 2017 at 13:05, David Rowley <david.rowley@2ndquadrant.com> wrote:
>> > I tested with the attached, and it does not seem to hurt planner
>> > performance executing:
>>
>> Here's it again, this time with a comment on the
>> find_relation_from_clauses() function.
>
> It seems to work as the same as the previous version with
> additional cost to scan over restrict clauses. But separate loop
> over clauses is additional overhead in any cases even irrelavant
> to functional dependency.  The more columns are in the relation,
> the longer time bms_get_singleton_member takes. Although I'm not
> sure how much it hurts performance and I can't think of a good
> alternative right now, I think that the overhead should be
> avoided anyhow.

I'm not all that sure why the number of columns in the relation has
relevance to the performance of find_relation_from_clauses(). The
bms_get_singleton_member() is checking which relations are part of the
RestrictInfo, nothing related to columns in relations.

Perhaps you meant clauses in the clauses list? Which does not really
have all that much to do with the number of columns in the relation
either.

>
> At Thu, 6 Apr 2017 13:05:24 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f_gB=gyZn8wMw0v8uCKD1nYeWyNYCXKz=+Oo0yR4RRyiA@mail.gmail.com>
>> > And you measured the overhead of doing it the other way to be ... ?
>> > Premature optimization and all that.
>>
>> I tested with the attached, and it does not seem to hurt planner
>> performance executing:
>
> Here, bms_singleton_member takes longer time if the relation has
> many columns and there's a functional dependency covering the
> columns at the very tail. Maybe only two are not practical for
> testing.

Can you explain why you think this? And confirm you're speaking about
the bms_get_singleton() member in find_relation_from_clauses()

> Even if it doesn't impact performance detectably, if only one
> attribute is needed, an AttrNumber member in context will be
> sufficient. No bitmap operation seems required in
> dependency_compatible_walker and it can bail out by the second
> attribute.

Are you looking at an old patch? That function no longer exists.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  9. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2017-04-06T07:50:56Z

    At Thu, 6 Apr 2017 18:59:35 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f-yrLizV5N_-r1o4vemuZBTJd8EzwPyx2QG=F6891++=g@mail.gmail.com>
> On 6 April 2017 at 18:03, Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> > At Thu, 6 Apr 2017 13:10:48 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f8Um=BvRmgcb3u6ze1q1xL7A1VKTVF9s2R1_UfRqx8q5w@mail.gmail.com>
> >> On 6 April 2017 at 13:05, David Rowley <david.rowley@2ndquadrant.com> wrote:
> I'm not all that sure why the number of columns in the relation has
> relevance to the performance of find_relation_from_clauses(). The
> bms_get_singleton_member() is checking which relations are part of the
> RestrictInfo, nothing related to columns in relations.
> Perhaps you meant clauses in the clauses list? Which does not really
> have all that much to do with the number of columns in the relation
> either.

Sorry, it's number of relations, not columns. I'm not sure up to
how many relations we practically should consider but anyway it
is extra burden to every call to clauselist_selectivity. We
should avoid calling find_relation_from_clauses as far as
possible or do the same in more simple way. However I'm not sure
more precise exclusion is possible or not, I thinks that the case
of jointype != JOIN_INNER can be exluded.

> > At Thu, 6 Apr 2017 13:05:24 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f_gB=gyZn8wMw0v8uCKD1nYeWyNYCXKz=+Oo0yR4RRyiA@mail.gmail.com>
> >> > And you measured the overhead of doing it the other way to be ... ?
> >> > Premature optimization and all that.
> >>
> >> I tested with the attached, and it does not seem to hurt planner
> >> performance executing:
> >
> > Here, bms_singleton_member takes longer time if the relation has
> > many columns and there's a functional dependency covering the
> > columns at the very tail. Maybe only two are not practical for
> > testing.
> 
> Can you explain why you think this? And confirm you're speaking about
> the bms_get_singleton() member in find_relation_from_clauses()

I mentioned dependency_is_compatible_clause here, but I saw that
it has been simplified enough in the committed shape.

> > Even if it doesn't impact performance detectably, if only one
> > attribute is needed, an AttrNumber member in context will be
> > sufficient. No bitmap operation seems required in
> > dependency_compatible_walker and it can bail out by the second
> > attribute.
> 
> Are you looking at an old patch? That function no longer exists.

Yes! Sorry for the noise.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  10. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    David Rowley <david.rowley@2ndquadrant.com> — 2017-04-06T09:55:43Z

    On 6 April 2017 at 19:50, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> At Thu, 6 Apr 2017 18:59:35 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f-yrLizV5N_-r1o4vemuZBTJd8EzwPyx2QG=F6891++=g@mail.gmail.com>
>> On 6 April 2017 at 18:03, Kyotaro HORIGUCHI
>> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
>> > At Thu, 6 Apr 2017 13:10:48 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f8Um=BvRmgcb3u6ze1q1xL7A1VKTVF9s2R1_UfRqx8q5w@mail.gmail.com>
>> >> On 6 April 2017 at 13:05, David Rowley <david.rowley@2ndquadrant.com> wrote:
>> I'm not all that sure why the number of columns in the relation has
>> relevance to the performance of find_relation_from_clauses(). The
>> bms_get_singleton_member() is checking which relations are part of the
>> RestrictInfo, nothing related to columns in relations.
>> Perhaps you meant clauses in the clauses list? Which does not really
>> have all that much to do with the number of columns in the relation
>> either.
>
> Sorry, it's number of relations, not columns. I'm not sure up to
> how many relations we practically should consider but anyway it
> is extra burden to every call to clauselist_selectivity. We
> should avoid calling find_relation_from_clauses as far as
> possible or do the same in more simple way. However I'm not sure
> more precise exclusion is possible or not, I thinks that the case
> of jointype != JOIN_INNER can be exluded.

Well, I imagine queries with >= 32 relations are not planning very
quickly as of today already. I understand what you mean when you speak
of attributes, as we could constantly be looking for the 1400's
attribute which is many loops into a bms_get_singleton_member() call.
I can't imagine we'll even flow over the first word in a bitmap set in
99% of cases with clause_relids.  In any case, even if there's a giant
chain of clauses in the the 'clauses' list, we'll bail out on the
first join qual anyway, since it won't be a singleton clause_relid.

I'd say if you can come up with a test case where you can measure the
impact of this, then let's discuss more. Otherwise we're stepping back
into the territory that Tom warned me about a few emails up....
Premature Optimisation. I'm not walking down there again, I only just
got back.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  11. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2017-04-06T10:42:24Z

    At Thu, 6 Apr 2017 21:55:43 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f95tOuSEMfmYWBPj-fGw=SY0MYDbQh5BiRiTtonMpws7Q@mail.gmail.com>
> On 6 April 2017 at 19:50, Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> > At Thu, 6 Apr 2017 18:59:35 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f-yrLizV5N_-r1o4vemuZBTJd8EzwPyx2QG=F6891++=g@mail.gmail.com>
> >> On 6 April 2017 at 18:03, Kyotaro HORIGUCHI
> >> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> >> > At Thu, 6 Apr 2017 13:10:48 +1200, David Rowley <david.rowley@2ndquadrant.com> wrote in <CAKJS1f8Um=BvRmgcb3u6ze1q1xL7A1VKTVF9s2R1_UfRqx8q5w@mail.gmail.com>
> >> >> On 6 April 2017 at 13:05, David Rowley <david.rowley@2ndquadrant.com> wrote:
> >> I'm not all that sure why the number of columns in the relation has
> >> relevance to the performance of find_relation_from_clauses(). The
> >> bms_get_singleton_member() is checking which relations are part of the
> >> RestrictInfo, nothing related to columns in relations.
> >> Perhaps you meant clauses in the clauses list? Which does not really
> >> have all that much to do with the number of columns in the relation
> >> either.
> >
> > Sorry, it's number of relations, not columns. I'm not sure up to
> > how many relations we practically should consider but anyway it
> > is extra burden to every call to clauselist_selectivity. We
> > should avoid calling find_relation_from_clauses as far as
> > possible or do the same in more simple way. However I'm not sure
> > more precise exclusion is possible or not, I thinks that the case
> > of jointype != JOIN_INNER can be exluded.
> 
> Well, I imagine queries with >= 32 relations are not planning very
> quickly as of today already. I understand what you mean when you speak
> of attributes, as we could constantly be looking for the 1400's
> attribute which is many loops into a bms_get_singleton_member() call.
> I can't imagine we'll even flow over the first word in a bitmap set in
> 99% of cases with clause_relids.  In any case, even if there's a giant
> chain of clauses in the the 'clauses' list, we'll bail out on the
> first join qual anyway, since it won't be a singleton clause_relid.

Yes, I agree that most cases doesn't suffer this. Anyway since I
don't have enough knowlege required to roughly estimate the
impact nor concrete expample where the planning time increases
significantly, I don't assert any more on this point.

> I'd say if you can come up with a test case where you can measure the
> impact of this, then let's discuss more. Otherwise we're stepping back
> into the territory that Tom warned me about a few emails up....
> Premature Optimisation. I'm not walking down there again, I only just
> got back.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  12. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    Simon Riggs <simon@2ndquadrant.com> — 2017-04-06T12:47:50Z

    On 5 April 2017 at 18:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Simon Riggs <simon@2ndQuadrant.com> writes:
>> Collect and use multi-column dependency stats
>
> The buildfarm is unhappy about the fact that this changed the API
> for clauselist_selectivity().  I am not convinced that that change
> was a good idea, so before telling FDW authors that they need to
> change their code, I'd like to hear a defense of the API change.
> Why not just use the existing varRelid parameter for that?  Why
> is there an assumption that only one rel's extended stats will
> ever be of interest?  This function does get used for join clauses.

Point noted. Reading thread and hope to fix today.

-- 
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  13. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    David Rowley <david.rowley@2ndquadrant.com> — 2017-04-06T21:41:24Z

    On 7 April 2017 at 00:47, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 5 April 2017 at 18:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Simon Riggs <simon@2ndQuadrant.com> writes:
>>> Collect and use multi-column dependency stats
>>
>> The buildfarm is unhappy about the fact that this changed the API
>> for clauselist_selectivity().  I am not convinced that that change
>> was a good idea, so before telling FDW authors that they need to
>> change their code, I'd like to hear a defense of the API change.
>> Why not just use the existing varRelid parameter for that?  Why
>> is there an assumption that only one rel's extended stats will
>> ever be of interest?  This function does get used for join clauses.
>
> Point noted. Reading thread and hope to fix today.

I've attached a rebased patch which fixes up the conflict with the
BRIN cost estimate patch which went in a short while ago.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

  14. Re: [COMMITTERS] pgsql: Collect and use multi-column dependency stats

    Simon Riggs <simon@2ndquadrant.com> — 2017-04-06T23:14:22Z

    On 6 April 2017 at 17:41, David Rowley <david.rowley@2ndquadrant.com> wrote:
> On 7 April 2017 at 00:47, Simon Riggs <simon@2ndquadrant.com> wrote:
>> On 5 April 2017 at 18:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Simon Riggs <simon@2ndQuadrant.com> writes:
>>>> Collect and use multi-column dependency stats
>>>
>>> The buildfarm is unhappy about the fact that this changed the API
>>> for clauselist_selectivity().  I am not convinced that that change
>>> was a good idea, so before telling FDW authors that they need to
>>> change their code, I'd like to hear a defense of the API change.
>>> Why not just use the existing varRelid parameter for that?  Why
>>> is there an assumption that only one rel's extended stats will
>>> ever be of interest?  This function does get used for join clauses.
>>
>> Point noted. Reading thread and hope to fix today.
>
> I've attached a rebased patch which fixes up the conflict with the
> BRIN cost estimate patch which went in a short while ago.

Looks enough to me, for now at least. Minor comment added.

-- 
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services