Horton

Thread

Commits

  1. Allow setting statistics target for extended statistics

  1. Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-06-18T21:33:57Z

    Hi,

One slightly inconvenient thing I realized while playing with the
address data set is that it's somewhat difficult to set the desired size
of the multi-column MCV list.

At the moment, we simply use the maximum statistic target for attributes
the MCV list is built on. But that does not allow keeping default size
for per-column stats, and only increase size of multi-column MCV lists.

So I'm thinking we should allow tweaking the statistics for extended
stats, and serialize it in the pg_statistic_ext catalog. Any opinions
why that would be a bad idea?

I suppose it should be part of the CREATE STATISTICS command, but I'm
not sure what'd be the best syntax. We might also have something more
similar to ALTER COLUMNT, but perhaps

     ALTER STATISTICS s SET STATISTICS 1000;

looks a bit too weird.


regards

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

  2. Re: Multivariate MCV list vs. statistics target

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2019-06-20T07:08:44Z

    On Tue, 18 Jun 2019 at 22:34, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>
> One slightly inconvenient thing I realized while playing with the
> address data set is that it's somewhat difficult to set the desired size
> of the multi-column MCV list.
>
> At the moment, we simply use the maximum statistic target for attributes
> the MCV list is built on. But that does not allow keeping default size
> for per-column stats, and only increase size of multi-column MCV lists.
>
> So I'm thinking we should allow tweaking the statistics for extended
> stats, and serialize it in the pg_statistic_ext catalog. Any opinions
> why that would be a bad idea?
>

Seems reasonable to me. This might not be the only option we'll ever
want to add though, so perhaps a "stxoptions text[]" column along the
lines of a relation's reloptions would be the way to go.

> I suppose it should be part of the CREATE STATISTICS command, but I'm
> not sure what'd be the best syntax. We might also have something more
> similar to ALTER COLUMNT, but perhaps
>
>      ALTER STATISTICS s SET STATISTICS 1000;
>
> looks a bit too weird.
>

Yes it does look a bit weird, but that's the natural generalisation of
what we have for per-column statistics, so it's probably preferable to
do that rather than invent some other syntax that wouldn't be so
consistent.

Regards,
Dean

  3. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-06-20T22:12:50Z

    On Thu, Jun 20, 2019 at 08:08:44AM +0100, Dean Rasheed wrote:
>On Tue, 18 Jun 2019 at 22:34, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>>
>> One slightly inconvenient thing I realized while playing with the
>> address data set is that it's somewhat difficult to set the desired size
>> of the multi-column MCV list.
>>
>> At the moment, we simply use the maximum statistic target for attributes
>> the MCV list is built on. But that does not allow keeping default size
>> for per-column stats, and only increase size of multi-column MCV lists.
>>
>> So I'm thinking we should allow tweaking the statistics for extended
>> stats, and serialize it in the pg_statistic_ext catalog. Any opinions
>> why that would be a bad idea?
>>
>
>Seems reasonable to me. This might not be the only option we'll ever
>want to add though, so perhaps a "stxoptions text[]" column along the
>lines of a relation's reloptions would be the way to go.
>

I don't know - I kinda dislike the idea of stashing stuff like this into
text[] arrays unless there's a clear need for such flexibility (i.e.
vision to have more such options). Which I'm not sure is the case here.
And we kinda have a precedent in pg_attribute.attstattarget, so I'd use
the same approach here.

>> I suppose it should be part of the CREATE STATISTICS command, but I'm
>> not sure what'd be the best syntax. We might also have something more
>> similar to ALTER COLUMNT, but perhaps
>>
>>      ALTER STATISTICS s SET STATISTICS 1000;
>>
>> looks a bit too weird.
>>
>
>Yes it does look a bit weird, but that's the natural generalisation of
>what we have for per-column statistics, so it's probably preferable to
>do that rather than invent some other syntax that wouldn't be so
>consistent.
>

Yeah, I agree.

regards

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

  4. Re: Multivariate MCV list vs. statistics target

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2019-06-21T07:09:18Z

    On Thu, 20 Jun 2019 at 23:12, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>
> On Thu, Jun 20, 2019 at 08:08:44AM +0100, Dean Rasheed wrote:
> >On Tue, 18 Jun 2019 at 22:34, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
> >>
> >> So I'm thinking we should allow tweaking the statistics for extended
> >> stats, and serialize it in the pg_statistic_ext catalog. Any opinions
> >> why that would be a bad idea?
> >
> >Seems reasonable to me. This might not be the only option we'll ever
> >want to add though, so perhaps a "stxoptions text[]" column along the
> >lines of a relation's reloptions would be the way to go.
>
> I don't know - I kinda dislike the idea of stashing stuff like this into
> text[] arrays unless there's a clear need for such flexibility (i.e.
> vision to have more such options). Which I'm not sure is the case here.
> And we kinda have a precedent in pg_attribute.attstattarget, so I'd use
> the same approach here.
>

Hmm, maybe. I can certainly understand your dislike of using text[].
I'm not sure that we can confidently say that multivariate statistics
won't ever need additional tuning knobs, but I have no idea at the
moment what they might be, and nothing else has come up so far in all
the time spent considering MCV lists and histograms, so maybe this is
OK.

Regards,
Dean

  5. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-06-29T10:41:21Z

    On Fri, Jun 21, 2019 at 08:09:18AM +0100, Dean Rasheed wrote:
>On Thu, 20 Jun 2019 at 23:12, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>>
>> On Thu, Jun 20, 2019 at 08:08:44AM +0100, Dean Rasheed wrote:
>> >On Tue, 18 Jun 2019 at 22:34, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>> >>
>> >> So I'm thinking we should allow tweaking the statistics for extended
>> >> stats, and serialize it in the pg_statistic_ext catalog. Any opinions
>> >> why that would be a bad idea?
>> >
>> >Seems reasonable to me. This might not be the only option we'll ever
>> >want to add though, so perhaps a "stxoptions text[]" column along the
>> >lines of a relation's reloptions would be the way to go.
>>
>> I don't know - I kinda dislike the idea of stashing stuff like this into
>> text[] arrays unless there's a clear need for such flexibility (i.e.
>> vision to have more such options). Which I'm not sure is the case here.
>> And we kinda have a precedent in pg_attribute.attstattarget, so I'd use
>> the same approach here.
>>
>
>Hmm, maybe. I can certainly understand your dislike of using text[].
>I'm not sure that we can confidently say that multivariate statistics
>won't ever need additional tuning knobs, but I have no idea at the
>moment what they might be, and nothing else has come up so far in all
>the time spent considering MCV lists and histograms, so maybe this is
>OK.
>

OK, attached is a patch implementing this - it adds

    ALTER STATISTICS ... SET STATISTICS ...

modifying a new stxstattarget column in pg_statistic_ext catalog,
following the same logic as pg_attribute.attstattarget.

During analyze, the per-ext-statistic value is determined like this:

1) When pg_statistic_ext.stxstattarget != (-1), we just use this value
and we're done.

2) Otherwise we inspect per-column attstattarget values, and use the
largest value. This is what we do now, so it's backwards-compatible
behavior.

3) If the value is still (-1), we use default_statistics_target.



regards

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

  6. Re: Multivariate MCV list vs. statistics target

    David Rowley <david.rowley@2ndquadrant.com> — 2019-06-30T07:34:32Z

    On Fri, 21 Jun 2019 at 19:09, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> Hmm, maybe. I can certainly understand your dislike of using text[].
> I'm not sure that we can confidently say that multivariate statistics
> won't ever need additional tuning knobs, but I have no idea at the
> moment what they might be, and nothing else has come up so far in all
> the time spent considering MCV lists and histograms, so maybe this is
> OK.

I agree with having the stxstattarget column. Even if something did
come up in the future, then we could consider merging the
stxstattarget column with a new text[] column at that time.

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

  7. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-07-06T22:02:38Z

    Hi,

apparently v1 of the ALTER STATISTICS patch was a bit confused about
length of the VacAttrStats array passed to statext_compute_stattarget,
causing segfaults. Attached v2 patch fixes that, and it also makes sure
we print warnings about ignored statistics only once.


regards

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

  8. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-07-08T18:18:14Z

    On Sun, Jul 07, 2019 at 12:02:38AM +0200, Tomas Vondra wrote:
>Hi,
>
>apparently v1 of the ALTER STATISTICS patch was a bit confused about
>length of the VacAttrStats array passed to statext_compute_stattarget,
>causing segfaults. Attached v2 patch fixes that, and it also makes sure
>we print warnings about ignored statistics only once.
>

v3 of the patch, adding pg_dump support - it works just like when you
tweak statistics target for a column, for example. When the value stored
in the catalog is not -1, pg_dump emits a separate ALTER STATISTICS
command setting it (for the already created statistics object).

I've considered making it part of CREATE STATISTICS itself, but it seems
a bit cumbersome and we don't do it for columns either. I'm not against
adding it in the future, but at this point I don't see a need.

At this point I'm not aware of any missing or broken pieces, so I'd
welcome feedback.

regards

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

  9. RE: Multivariate MCV list vs. statistics target

    Jamison, Kirk <k.jamison@jp.fujitsu.com> — 2019-07-19T06:12:20Z

    On Tuesday, July 9, 2019, Tomas Vondra wrote:
> >apparently v1 of the ALTER STATISTICS patch was a bit confused about
> >length of the VacAttrStats array passed to statext_compute_stattarget,
> >causing segfaults. Attached v2 patch fixes that, and it also makes sure
> >we print warnings about ignored statistics only once.
> >
> 
> v3 of the patch, adding pg_dump support - it works just like when you tweak
> statistics target for a column, for example. When the value stored in the
> catalog is not -1, pg_dump emits a separate ALTER STATISTICS command setting
> it (for the already created statistics object).
> 

Hi Tomas, I stumbled upon your patch.

According to the CF bot, your patch applies cleanly, builds successfully, and
passes make world. Meaning, the pg_dump tap test passed, but there was no
test for the new SET STATISTICS yet. So you might want to add a regression
test for that and integrate it in the existing alter_generic file.

Upon quick read-through, the syntax and docs are correct because it's similar
to the format of ALTER TABLE/INDEX... SET STATISTICS... :
    ALTER [ COLUMN ] column_name SET STATISTICS integer

+		/* XXX What if the target is set to 0? Reset the statistic? */

This also makes me wonder. I haven't looked deeply into the code, but since 0 is
a valid value, I believe it should reset the stats.
After lookup though, this is how it's tested in ALTER TABLE:
/test/regress/sql/stats_ext.sql:-- Ensure things work sanely with SET STATISTICS 0

> I've considered making it part of CREATE STATISTICS itself, but it seems a
> bit cumbersome and we don't do it for columns either. I'm not against adding
> it in the future, but at this point I don't see a need.

I agree. Perhaps that's for another patch should you decide to add it in the future.

Regards,
Kirk Jamison

  10. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-07-19T23:12:21Z

    On Fri, Jul 19, 2019 at 06:12:20AM +0000, Jamison, Kirk wrote:
>On Tuesday, July 9, 2019, Tomas Vondra wrote:
>> >apparently v1 of the ALTER STATISTICS patch was a bit confused about
>> >length of the VacAttrStats array passed to statext_compute_stattarget,
>> >causing segfaults. Attached v2 patch fixes that, and it also makes sure
>> >we print warnings about ignored statistics only once.
>> >
>>
>> v3 of the patch, adding pg_dump support - it works just like when you tweak
>> statistics target for a column, for example. When the value stored in the
>> catalog is not -1, pg_dump emits a separate ALTER STATISTICS command setting
>> it (for the already created statistics object).
>>
>
>Hi Tomas, I stumbled upon your patch.
>
>According to the CF bot, your patch applies cleanly, builds successfully, and
>passes make world. Meaning, the pg_dump tap test passed, but there was no
>test for the new SET STATISTICS yet. So you might want to add a regression
>test for that and integrate it in the existing alter_generic file.
>
>Upon quick read-through, the syntax and docs are correct because it's similar
>to the format of ALTER TABLE/INDEX... SET STATISTICS... :
>    ALTER [ COLUMN ] column_name SET STATISTICS integer
>
>+		/* XXX What if the target is set to 0? Reset the statistic? */
>
>This also makes me wonder. I haven't looked deeply into the code, but since 0 is
>a valid value, I believe it should reset the stats.

I agree resetting the stats after setting the target to 0 seems quite
reasonable. But that's not what we do for attribute stats, because in
that case we simply skip the attribute during the future ANALYZE runs -
we don't reset the stats, we keep the existing stats. So I've done the
same thing here, and I've removed the XXX comment.

If we want to change that, I'd do it in a separate patch for both the
regular and extended stats.

>After lookup though, this is how it's tested in ALTER TABLE:
>/test/regress/sql/stats_ext.sql:-- Ensure things work sanely with SET STATISTICS 0
>

Well, yeah. But that tests we skip building the extended statistic
(because we excluded the column from the ANALYZE run).

>> I've considered making it part of CREATE STATISTICS itself, but it seems a
>> bit cumbersome and we don't do it for columns either. I'm not against adding
>> it in the future, but at this point I don't see a need.
>
>I agree. Perhaps that's for another patch should you decide to add it in the future.
>

Right.

Attached is v4 of the patch, with a couple more improvements:

1) I've renamed the if_not_exists flag to missing_ok, because that's
more consistent with the "IF EXISTS" clause in the grammar (the old flag
was kinda the exact opposite), and I've added a NOTICE about the skip.

2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows, because
that's what the function was doing anyway (computing sample size).

3) I've added a couple of regression tests to stats_ext.sql

Aside from that, I've cleaned up a couple of places and improved a bunch
of comments. Nothing huge.


regards

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

  11. RE: Multivariate MCV list vs. statistics target

    Jamison, Kirk <k.jamison@jp.fujitsu.com> — 2019-07-26T07:03:41Z

    On Sat, July 20, 2019 8:12 AM (GMT+9), Tomas Vondra wrote:

> >+		/* XXX What if the target is set to 0? Reset the statistic?
> */
> >
> >This also makes me wonder. I haven't looked deeply into the code, but
> >since 0 is a valid value, I believe it should reset the stats.
> 
> I agree resetting the stats after setting the target to 0 seems quite
> reasonable. But that's not what we do for attribute stats, because in that
> case we simply skip the attribute during the future ANALYZE runs - we don't
> reset the stats, we keep the existing stats. So I've done the same thing here,
> and I've removed the XXX comment.
> 
> If we want to change that, I'd do it in a separate patch for both the regular
> and extended stats.

Hi, Tomas

Sorry for my late reply.
You're right. I have no strong opinion whether we'd want to change that behavior.
I've also confirmed the change in the patch where setting statistics target 0
skips the statistics. 

Maybe only some minor nitpicks in the source code comments below:
1. "it's" should be "its":
> +		 * Compute statistic target, based on what's set for the statistic
> +		 * object itself, and for it's attributes.

2. Consistency whether you'd use either "statistic " or "statisticS ".
Ex. statistic target vs statisticS target, statistics object vs statistic object, etc.

> Attached is v4 of the patch, with a couple more improvements:
>
> 1) I've renamed the if_not_exists flag to missing_ok, because that's more
> consistent with the "IF EXISTS" clause in the grammar (the old flag was kinda
> the exact opposite), and I've added a NOTICE about the skip.

+	bool		missing_ok;      /* do nothing if statistics does not exist */

Confirmed. So we ignore if statistic does not exist, and skip the error.
Maybe to make it consistent with other data structures in parsernodes.h,
you can change the comment of missing_ok to: 
/* skip error if statistics object does not exist */

> 2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows, because that's
> what the function was doing anyway (computing sample size).
>
> 3) I've added a couple of regression tests to stats_ext.sql
> 
> Aside from that, I've cleaned up a couple of places and improved a bunch of
> comments. Nothing huge.

I have a question though regarding ComputeExtStatisticsRows.
I'm just curious with the value 300 when computing sample size.
Where did this value come from?

+	/* compute sample size based on the statistic target */
+	return (300 * result);

Overall, the patch is almost already in good shape for commit.
I'll wait for the next update.

Regards,
Kirk Jamison

  12. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-07-26T22:05:52Z

    On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
>On Sat, July 20, 2019 8:12 AM (GMT+9), Tomas Vondra wrote:
>
>> >+		/* XXX What if the target is set to 0? Reset the statistic?
>> */
>> >
>> >This also makes me wonder. I haven't looked deeply into the code, but
>> >since 0 is a valid value, I believe it should reset the stats.
>>
>> I agree resetting the stats after setting the target to 0 seems quite
>> reasonable. But that's not what we do for attribute stats, because in that
>> case we simply skip the attribute during the future ANALYZE runs - we don't
>> reset the stats, we keep the existing stats. So I've done the same thing here,
>> and I've removed the XXX comment.
>>
>> If we want to change that, I'd do it in a separate patch for both the regular
>> and extended stats.
>
>Hi, Tomas
>
>Sorry for my late reply.
>You're right. I have no strong opinion whether we'd want to change that behavior.
>I've also confirmed the change in the patch where setting statistics target 0
>skips the statistics.
>

OK, thanks.

>Maybe only some minor nitpicks in the source code comments below:
>1. "it's" should be "its":
>> +		 * Compute statistic target, based on what's set for the statistic
>> +		 * object itself, and for it's attributes.
>
>2. Consistency whether you'd use either "statistic " or "statisticS ".
>Ex. statistic target vs statisticS target, statistics object vs statistic object, etc.
>
>> Attached is v4 of the patch, with a couple more improvements:
>>
>> 1) I've renamed the if_not_exists flag to missing_ok, because that's more
>> consistent with the "IF EXISTS" clause in the grammar (the old flag was kinda
>> the exact opposite), and I've added a NOTICE about the skip.
>
>+	bool		missing_ok;      /* do nothing if statistics does not exist */
>
>Confirmed. So we ignore if statistic does not exist, and skip the error.
>Maybe to make it consistent with other data structures in parsernodes.h,
>you can change the comment of missing_ok to:
>/* skip error if statistics object does not exist */
>

Thanks, I've fixed all those places in the attached v5.

>> 2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows, because that's
>> what the function was doing anyway (computing sample size).
>>
>> 3) I've added a couple of regression tests to stats_ext.sql
>>
>> Aside from that, I've cleaned up a couple of places and improved a bunch of
>> comments. Nothing huge.
>
>I have a question though regarding ComputeExtStatisticsRows.
>I'm just curious with the value 300 when computing sample size.
>Where did this value come from?
>
>+	/* compute sample size based on the statistic target */
>+	return (300 * result);
>
>Overall, the patch is almost already in good shape for commit.
>I'll wait for the next update.
>

That's how we compute number of rows to sample, based on the statistics
target. See std_typanalyze() in analyze.c, which also cites the paper
where this comes from.

regards

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

  13. RE: Multivariate MCV list vs. statistics target

    Jamison, Kirk <k.jamison@jp.fujitsu.com> — 2019-07-29T01:53:08Z

    On Saturday, July 27, 2019 7:06 AM(GMT+9), Tomas Vondra wrote:
> On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
> >On Sat, July 20, 2019 8:12 AM (GMT+9), Tomas Vondra wrote:
> >
> >> >+		/* XXX What if the target is set to 0? Reset the statistic?
> >> */
> >> >
> >> >This also makes me wonder. I haven't looked deeply into the code,
> >> >but since 0 is a valid value, I believe it should reset the stats.
> >>
> >> I agree resetting the stats after setting the target to 0 seems quite
> >> reasonable. But that's not what we do for attribute stats, because in
> >> that case we simply skip the attribute during the future ANALYZE runs
> >> - we don't reset the stats, we keep the existing stats. So I've done
> >> the same thing here, and I've removed the XXX comment.
> >>
> >> If we want to change that, I'd do it in a separate patch for both the
> >> regular and extended stats.
> >
> >Hi, Tomas
> >
> >Sorry for my late reply.
> >You're right. I have no strong opinion whether we'd want to change that
> behavior.
> >I've also confirmed the change in the patch where setting statistics
> >target 0 skips the statistics.
> >
> 
> OK, thanks.
> 
> >Maybe only some minor nitpicks in the source code comments below:
> >1. "it's" should be "its":
> >> +		 * Compute statistic target, based on what's set for the
> statistic
> >> +		 * object itself, and for it's attributes.
> >
> >2. Consistency whether you'd use either "statistic " or "statisticS ".
> >Ex. statistic target vs statisticS target, statistics object vs statistic
> object, etc.
> >
> >> Attached is v4 of the patch, with a couple more improvements:
> >>
> >> 1) I've renamed the if_not_exists flag to missing_ok, because that's
> >> more consistent with the "IF EXISTS" clause in the grammar (the old
> >> flag was kinda the exact opposite), and I've added a NOTICE about the skip.
> >
> >+	bool		missing_ok;      /* do nothing if statistics does
> not exist */
> >
> >Confirmed. So we ignore if statistic does not exist, and skip the error.
> >Maybe to make it consistent with other data structures in
> >parsernodes.h, you can change the comment of missing_ok to:
> >/* skip error if statistics object does not exist */
> >
> 
> Thanks, I've fixed all those places in the attached v5.

I've confirmed the fix.

> >> 2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows, because
> >> that's what the function was doing anyway (computing sample size).
> >>
> >> 3) I've added a couple of regression tests to stats_ext.sql
> >>
> >> Aside from that, I've cleaned up a couple of places and improved a
> >> bunch of comments. Nothing huge.
> >
> >I have a question though regarding ComputeExtStatisticsRows.
> >I'm just curious with the value 300 when computing sample size.
> >Where did this value come from?
> >
> >+	/* compute sample size based on the statistic target */
> >+	return (300 * result);
> >
> >Overall, the patch is almost already in good shape for commit.
> >I'll wait for the next update.
> >
> 
> That's how we compute number of rows to sample, based on the statistics target.
> See std_typanalyze() in analyze.c, which also cites the paper where this comes
> from.
Noted. Found it. Thank you for the reference.


There's just a small whitespace (extra space) below after running git diff --check.
>src/bin/pg_dump/pg_dump.c:7226: trailing whitespace.
>+ 
It would be better to post an updated patch,
but other than that, I've confirmed the fixes.
The patch passed the make-world and regression tests as well.
I've marked this as "ready for committer".


Regards,
Kirk Jamison

  14. RE: Multivariate MCV list vs. statistics target

    Jamison, Kirk <k.jamison@jp.fujitsu.com> — 2019-08-01T00:15:48Z

    Hi, 

> From: Jamison, Kirk [mailto:k.jamison@jp.fujitsu.com]
> Sent: Monday, July 29, 2019 10:53 AM
> To: 'Tomas Vondra' <tomas.vondra@2ndquadrant.com>
> Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers
> <pgsql-hackers@lists.postgresql.org>
> Subject: RE: Multivariate MCV list vs. statistics target
> 
> On Saturday, July 27, 2019 7:06 AM(GMT+9), Tomas Vondra wrote:
> > On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
> > >On Sat, July 20, 2019 8:12 AM (GMT+9), Tomas Vondra wrote:
> > >
> > >> >+		/* XXX What if the target is set to 0? Reset the statistic?
> > >> */
> > >> >
> > >> >This also makes me wonder. I haven't looked deeply into the code,
> > >> >but since 0 is a valid value, I believe it should reset the stats.
> > >>
> > >> I agree resetting the stats after setting the target to 0 seems
> > >> quite reasonable. But that's not what we do for attribute stats,
> > >> because in that case we simply skip the attribute during the future
> > >> ANALYZE runs
> > >> - we don't reset the stats, we keep the existing stats. So I've
> > >> done the same thing here, and I've removed the XXX comment.
> > >>
> > >> If we want to change that, I'd do it in a separate patch for both
> > >> the regular and extended stats.
> > >
> > >Hi, Tomas
> > >
> > >Sorry for my late reply.
> > >You're right. I have no strong opinion whether we'd want to change
> > >that
> > behavior.
> > >I've also confirmed the change in the patch where setting statistics
> > >target 0 skips the statistics.
> > >
> >
> > OK, thanks.
> >
> > >Maybe only some minor nitpicks in the source code comments below:
> > >1. "it's" should be "its":
> > >> +		 * Compute statistic target, based on what's set for the
> > statistic
> > >> +		 * object itself, and for it's attributes.
> > >
> > >2. Consistency whether you'd use either "statistic " or "statisticS ".
> > >Ex. statistic target vs statisticS target, statistics object vs
> > >statistic
> > object, etc.
> > >
> > >> Attached is v4 of the patch, with a couple more improvements:
> > >>
> > >> 1) I've renamed the if_not_exists flag to missing_ok, because
> > >> that's more consistent with the "IF EXISTS" clause in the grammar
> > >> (the old flag was kinda the exact opposite), and I've added a NOTICE
> about the skip.
> > >
> > >+	bool		missing_ok;      /* do nothing if statistics does
> > not exist */
> > >
> > >Confirmed. So we ignore if statistic does not exist, and skip the error.
> > >Maybe to make it consistent with other data structures in
> > >parsernodes.h, you can change the comment of missing_ok to:
> > >/* skip error if statistics object does not exist */
> > >
> >
> > Thanks, I've fixed all those places in the attached v5.
> 
> I've confirmed the fix.
> 
> > >> 2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows,
> > >> because that's what the function was doing anyway (computing sample
> size).
> > >>
> > >> 3) I've added a couple of regression tests to stats_ext.sql
> > >>
> > >> Aside from that, I've cleaned up a couple of places and improved a
> > >> bunch of comments. Nothing huge.
> > >
> > >I have a question though regarding ComputeExtStatisticsRows.
> > >I'm just curious with the value 300 when computing sample size.
> > >Where did this value come from?
> > >
> > >+	/* compute sample size based on the statistic target */
> > >+	return (300 * result);
> > >
> > >Overall, the patch is almost already in good shape for commit.
> > >I'll wait for the next update.
> > >
> >
> > That's how we compute number of rows to sample, based on the statistics
> target.
> > See std_typanalyze() in analyze.c, which also cites the paper where
> > this comes from.
> Noted. Found it. Thank you for the reference.
> 
> 
> There's just a small whitespace (extra space) below after running git diff
> --check.
> >src/bin/pg_dump/pg_dump.c:7226: trailing whitespace.
> >+
> It would be better to post an updated patch, but other than that, I've confirmed
> the fixes.
> The patch passed the make-world and regression tests as well.
> I've marked this as "ready for committer".

The patch does not apply anymore.
In addition to the whitespace detected,
kindly rebase the patch as there were changes from recent commits
in the following files:
       src/backend/commands/analyze.c
       src/bin/pg_dump/pg_dump.c
       src/bin/pg_dump/t/002_pg_dump.pl
       src/test/regress/expected/stats_ext.out
       src/test/regress/sql/stats_ext.sql

Regards,
Kirk Jamison

  15. Re: Multivariate MCV list vs. statistics target

    Thomas Munro <thomas.munro@gmail.com> — 2019-08-01T05:25:31Z

    On Thu, Aug 1, 2019 at 12:16 PM Jamison, Kirk <k.jamison@jp.fujitsu.com> wrote:
> > On Saturday, July 27, 2019 7:06 AM(GMT+9), Tomas Vondra wrote:
> > > On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
> > > >Overall, the patch is almost already in good shape for commit.
> > > >I'll wait for the next update.

> > The patch passed the make-world and regression tests as well.
> > I've marked this as "ready for committer".
>
> The patch does not apply anymore.

Based on the above, it sounds like this patch is super close and the
only problem is bitrot, so I've set it back to Ready for Committer.
Over to Tomas to rebase and commit, or move to the next CF if that's
more appropriate.

-- 
Thomas Munro
https://enterprisedb.com

  16. Re: Multivariate MCV list vs. statistics target

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2019-08-01T08:29:20Z

    Hello.

At Thu, 1 Aug 2019 00:15:48 +0000, "Jamison, Kirk" <k.jamison@jp.fujitsu.com> wrote in <D09B13F772D2274BB348A310EE3027C6518F94@g01jpexmbkw24>
> The patch does not apply anymore.
> In addition to the whitespace detected,
> kindly rebase the patch as there were changes from recent commits
> in the following files:
>        src/backend/commands/analyze.c
>        src/bin/pg_dump/pg_dump.c
>        src/bin/pg_dump/t/002_pg_dump.pl
>        src/test/regress/expected/stats_ext.out
>        src/test/regress/sql/stats_ext.sql

The patch finally failed only for stats_ext.out, where 14ef15a222
is hitting. (for b2a3d706b8)

I looked through this patch and have some comments.



+++ b/src/backend/commands/statscmds.c
+#include "access/heapam.h"
..
+#include "utils/fmgroids.h"

These don't seem needed.


+    Assert(stmt->missing_ok);

Perhaps we shouldn't Assert on this condition. Isn't it better we
just "elog(ERROR" here?


+    DeconstructQualifiedName(stmt->defnames, &schemaname, &statname);

Maybe we don't need detailed analysis that the function emits on
error. Couldn't we use NameListToString() instead? That reduces
the number of ereport()s and considerably simplify the code
around.

+  oldtup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(stxoid));
+
+  /* Must be owner of the existing statistics object */
+  if (!pg_statistics_object_ownercheck(stxoid, GetUserId()))

This repeats the SearchSysCache twice in a quite short
duration. I suppose it'd be better that ACL (and validity) checks
done directly using oldtup.


+  /* replace the stxstattarget column */
+  repl_repl[Anum_pg_statistic_ext_stxstattarget - 1] = true;
+  repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = Int32GetDatum(newtarget)

We usually do this kind of work using SearchSysCacheCopyN(),
which simplifies the code around, too.


+++ b/src/backend/statistics/mcv.c
>    * Maximum number of MCV items to store, based on the attribute with the
>    * largest stats target (and the number of groups we have available).
>    */
-  nitems = stats[0]->attr->attstattarget;
-  for (i = 1; i < numattrs; i++)
-  {
-    if (stats[i]->attr->attstattarget > nitems)
-      nitems = stats[i]->attr->attstattarget;
-  }
+  nitems = stattarget;

Maybe you forgot to modify the comment.


check_xact_readonly() returns false for this command. As the
result it emits a somewhat pointless error message.

=# alter statistics s1 set statistics 0;
ERROR:  cannot assign TransactionIds during recovery


+++ b/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.h
>   i_stxname = PQfnumber(res, "stxname");
>   i_stxnamespace = PQfnumber(res, "stxnamespace");
>   i_rolname = PQfnumber(res, "rolname");
+   i_stattarget = PQfnumber(res, "stxstattarget");

I'm not sure whether it is the convention here, but variable name
is different from column name only for the added line.


+++ b/src/bin/psql/tab-complete.c
-		COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA");
+		COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA", "SET STATISTICS");

ALTER STATISTICS s2 SET STATISTICS<tab> is suggested with
ext-stats names,  but it's the place for target value.


+++ b/src/include/nodes/nodes.h
   T_CallStmt,
+  T_AlterStatsStmt,
 
I think it should be immediately below T_CreateStatsStmt.


+++ b/src/include/nodes/parsenodes.h
+  bool    missing_ok;    /* skip error if statistics object is missing */

Should be very trivial, but many bool members especially
missing_ok have a comment having "?" at the end.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  17. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-08-01T10:30:10Z

    On Thu, Aug 01, 2019 at 05:25:31PM +1200, Thomas Munro wrote:
>On Thu, Aug 1, 2019 at 12:16 PM Jamison, Kirk <k.jamison@jp.fujitsu.com> wrote:
>> > On Saturday, July 27, 2019 7:06 AM(GMT+9), Tomas Vondra wrote:
>> > > On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
>> > > >Overall, the patch is almost already in good shape for commit.
>> > > >I'll wait for the next update.
>
>> > The patch passed the make-world and regression tests as well.
>> > I've marked this as "ready for committer".
>>
>> The patch does not apply anymore.
>
>Based on the above, it sounds like this patch is super close and the
>only problem is bitrot, so I've set it back to Ready for Committer.
>Over to Tomas to rebase and commit, or move to the next CF if that's
>more appropriate.
>

I'll move it to the next CF. Aside from the issues pointed by Kyotaro-san
in his review, I still haven't made my mind about whether to base the use
statistics targets set for the attributes. That's what we're doing now,
but I'm not sure it's a good idea after adding separate statistics target.
I wonder what Dean's opinion on this is, as he added the current logic.

regards

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

  18. Re: Multivariate MCV list vs. statistics target

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2019-08-01T14:42:35Z

    On Thu, 1 Aug 2019 at 11:30, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>
> I'll move it to the next CF. Aside from the issues pointed by Kyotaro-san
> in his review, I still haven't made my mind about whether to base the use
> statistics targets set for the attributes. That's what we're doing now,
> but I'm not sure it's a good idea after adding separate statistics target.
> I wonder what Dean's opinion on this is, as he added the current logic.
>

If this were being released in the same version as MCV stats first
appeared, I'd say that there's not much point basing the default
multivariate stats target on the per-column targets, when it has its
own knob to control it. However, since this won't be released for a
year, those per-column-based defaults will be in the field for that
long, and so I'd say that we shouldn't change the default when adding
this, otherwise users who don't use this new feature might be
surprised by the change in behaviour.

Regards,
Dean

  19. Re: Multivariate MCV list vs. statistics target

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-09-03T18:38:56Z

    On 2019-Aug-01, Tomas Vondra wrote:

> I'll move it to the next CF. Aside from the issues pointed by Kyotaro-san
> in his review, I still haven't made my mind about whether to base the use
> statistics targets set for the attributes. That's what we're doing now,
> but I'm not sure it's a good idea after adding separate statistics target.
> I wonder what Dean's opinion on this is, as he added the current logic.

Latest patch no longer applies.  Please update.  And since you already
seem to have handled all review comments since it was Ready for
Committer, and you now know Dean's opinion on the remaining question,
please get it pushed.

Thanks

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  20. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2019-09-10T22:28:12Z

    On Tue, Sep 03, 2019 at 02:38:56PM -0400, Alvaro Herrera wrote:
>On 2019-Aug-01, Tomas Vondra wrote:
>
>> I'll move it to the next CF. Aside from the issues pointed by Kyotaro-san
>> in his review, I still haven't made my mind about whether to base the use
>> statistics targets set for the attributes. That's what we're doing now,
>> but I'm not sure it's a good idea after adding separate statistics target.
>> I wonder what Dean's opinion on this is, as he added the current logic.
>
>Latest patch no longer applies.  Please update.  And since you already
>seem to have handled all review comments since it was Ready for
>Committer, and you now know Dean's opinion on the remaining question,
>please get it pushed.
>

OK, I've pushed this the patch, after some minor polishing.


regards

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

  21. RE: Multivariate MCV list vs. statistics target

    Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com> — 2020-03-18T04:28:34Z

    Hello,

I found a missing column description in the pg_statistic_ext catalog document for this new feature.
The attached patch adds a description of the 'stxstattarget' column to pg_statistic_ext catalog's document. 
If there is a better explanation, please correct it.

Regards,
Noriyoshi Shinoda

-----Original Message-----
From: Tomas Vondra [mailto:tomas.vondra@2ndquadrant.com] 
Sent: Wednesday, September 11, 2019 7:28 AM
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Thomas Munro <thomas.munro@gmail.com>; Jamison, Kirk <k.jamison@jp.fujitsu.com>; Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Multivariate MCV list vs. statistics target

On Tue, Sep 03, 2019 at 02:38:56PM -0400, Alvaro Herrera wrote:
>On 2019-Aug-01, Tomas Vondra wrote:
>
>> I'll move it to the next CF. Aside from the issues pointed by 
>> Kyotaro-san in his review, I still haven't made my mind about whether 
>> to base the use statistics targets set for the attributes. That's 
>> what we're doing now, but I'm not sure it's a good idea after adding separate statistics target.
>> I wonder what Dean's opinion on this is, as he added the current logic.
>
>Latest patch no longer applies.  Please update.  And since you already 
>seem to have handled all review comments since it was Ready for 
>Committer, and you now know Dean's opinion on the remaining question, 
>please get it pushed.
>

OK, I've pushed this the patch, after some minor polishing.


regards

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

  22. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2020-03-18T12:36:29Z

    Hi,

On Wed, Mar 18, 2020 at 04:28:34AM +0000, Shinoda, Noriyoshi (PN Japan A&PS Delivery) wrote:
>Hello,
>
>I found a missing column description in the pg_statistic_ext catalog document for this new feature.
>The attached patch adds a description of the 'stxstattarget' column to pg_statistic_ext catalog's document.
>If there is a better explanation, please correct it.
>

Thanks for the report. Yes, this is clearly an omission. I think it
would be better to use wording similar to attstattarget, per the
attached patch. That is, without reference to ALTER STATISTICS and
better explaination of what positive/negative values do. Do you agree?


regards

>Regards,
>Noriyoshi Shinoda
>
>-----Original Message-----
>From: Tomas Vondra [mailto:tomas.vondra@2ndquadrant.com]
>Sent: Wednesday, September 11, 2019 7:28 AM
>To: Alvaro Herrera <alvherre@2ndquadrant.com>
>Cc: Thomas Munro <thomas.munro@gmail.com>; Jamison, Kirk <k.jamison@jp.fujitsu.com>; Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
>Subject: Re: Multivariate MCV list vs. statistics target
>
>On Tue, Sep 03, 2019 at 02:38:56PM -0400, Alvaro Herrera wrote:
>>On 2019-Aug-01, Tomas Vondra wrote:
>>
>>> I'll move it to the next CF. Aside from the issues pointed by
>>> Kyotaro-san in his review, I still haven't made my mind about whether
>>> to base the use statistics targets set for the attributes. That's
>>> what we're doing now, but I'm not sure it's a good idea after adding separate statistics target.
>>> I wonder what Dean's opinion on this is, as he added the current logic.
>>
>>Latest patch no longer applies.  Please update.  And since you already
>>seem to have handled all review comments since it was Ready for
>>Committer, and you now know Dean's opinion on the remaining question,
>>please get it pushed.
>>
>
>OK, I've pushed this the patch, after some minor polishing.
>
>
>regards
>
>-- 
>Tomas Vondra                  http://www.2ndQuadrant.com
>PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
>



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

  23. RE: Multivariate MCV list vs. statistics target

    Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com> — 2020-03-18T13:32:19Z

    Hi, 

>Thanks for the report. Yes, this is clearly an omission. I think it would be better to use wording >similar to attstattarget, per the attached patch. That is, without reference to ALTER STATISTICS and >better explaination of what positive/negative values do. Do you agree?

Thank you for your comment.
I agree with the text you suggested.

Regards,
Noriyoshi Shinoda

-----Original Message-----
From: Tomas Vondra [mailto:tomas.vondra@2ndquadrant.com] 
Sent: Wednesday, March 18, 2020 9:36 PM
To: Shinoda, Noriyoshi (PN Japan A&PS Delivery) <noriyoshi.shinoda@hpe.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Thomas Munro <thomas.munro@gmail.com>; Jamison, Kirk <k.jamison@jp.fujitsu.com>; Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Multivariate MCV list vs. statistics target

Hi,

On Wed, Mar 18, 2020 at 04:28:34AM +0000, Shinoda, Noriyoshi (PN Japan A&PS Delivery) wrote:
>Hello,
>
>I found a missing column description in the pg_statistic_ext catalog document for this new feature.
>The attached patch adds a description of the 'stxstattarget' column to pg_statistic_ext catalog's document.
>If there is a better explanation, please correct it.
>

Thanks for the report. Yes, this is clearly an omission. I think it would be better to use wording similar to attstattarget, per the attached patch. That is, without reference to ALTER STATISTICS and better explaination of what positive/negative values do. Do you agree?


regards

>Regards,
>Noriyoshi Shinoda
>
>-----Original Message-----
>From: Tomas Vondra [mailto:tomas.vondra@2ndquadrant.com]
>Sent: Wednesday, September 11, 2019 7:28 AM
>To: Alvaro Herrera <alvherre@2ndquadrant.com>
>Cc: Thomas Munro <thomas.munro@gmail.com>; Jamison, Kirk 
><k.jamison@jp.fujitsu.com>; Dean Rasheed <dean.a.rasheed@gmail.com>; 
>PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
>Subject: Re: Multivariate MCV list vs. statistics target
>
>On Tue, Sep 03, 2019 at 02:38:56PM -0400, Alvaro Herrera wrote:
>>On 2019-Aug-01, Tomas Vondra wrote:
>>
>>> I'll move it to the next CF. Aside from the issues pointed by 
>>> Kyotaro-san in his review, I still haven't made my mind about 
>>> whether to base the use statistics targets set for the attributes. 
>>> That's what we're doing now, but I'm not sure it's a good idea after adding separate statistics target.
>>> I wonder what Dean's opinion on this is, as he added the current logic.
>>
>>Latest patch no longer applies.  Please update.  And since you already 
>>seem to have handled all review comments since it was Ready for 
>>Committer, and you now know Dean's opinion on the remaining question, 
>>please get it pushed.
>>
>
>OK, I've pushed this the patch, after some minor polishing.
>
>
>regards
>
>-- 
>Tomas Vondra                  http://www.2ndQuadrant.com 
>PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
>



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

  24. Re: Multivariate MCV list vs. statistics target

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2020-03-18T15:53:26Z

    On Wed, Mar 18, 2020 at 01:32:19PM +0000, Shinoda, Noriyoshi (PN Japan
A&PS Delivery) wrote:
>Hi,
>
>>Thanks for the report. Yes, this is clearly an omission. I think it
>>would be better to use wording >similar to attstattarget, per the
>>attached patch. That is, without reference to ALTER STATISTICS and
>>>better explaination of what positive/negative values do. Do you
>>>agree?
>
>Thank you for your comment.  I agree with the text you suggested.
>

Thank you for the report, I've pushed the reworded fix.

regards

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

  25. Re: Multivariate MCV list vs. statistics target

    Justin Pryzby <pryzby@telsasoft.com> — 2020-09-06T01:12:31Z

    I think the docs are inconsistent with the commit message and the code
(d06215d03) and docs should be corrected, soemthing like so:

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index b135c89005..cd10a6a6fc 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -7302,7 +7302,8 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
        of statistics accumulated for this statistics object by
        <xref linkend="sql-analyze"/>.
        A zero value indicates that no statistics should be collected.
-       A negative value says to use the system default statistics target.
+       A negative value says to use the maximum of the statistics targets of
+       the referenced columns, if set, or the system default statistics target.
        Positive values of <structfield>stxstattarget</structfield>
        determine the target number of <quote>most common values</quote>
        to collect.
diff --git a/doc/src/sgml/ref/alter_statistics.sgml b/doc/src/sgml/ref/alter_statistics.sgml
index be4c3f1f05..f2e8a93166 100644
--- a/doc/src/sgml/ref/alter_statistics.sgml
+++ b/doc/src/sgml/ref/alter_statistics.sgml
@@ -101,7 +101,8 @@ ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET STATISTIC
         The statistic-gathering target for this statistics object for subsequent
         <xref linkend="sql-analyze"/> operations.
         The target can be set in the range 0 to 10000; alternatively, set it
-        to -1 to revert to using the system default statistics
+        to -1 to revert to using the maximum statistics target of the
+        referenced column's, if set, or the system default statistics
         target (<xref linkend="guc-default-statistics-target"/>).
         For more information on the use of statistics by the
         <productname>PostgreSQL</productname> query planner, refer to
-- 
2.17.0