Horton

Thread

Commits

  1. doc: Adjust documentation for vacuumdb --missing-stats-only.

  2. Update guidance for running vacuumdb after pg_upgrade.

  3. vacuumdb: Add option for analyzing only relations missing stats.

  4. vacuumdb: Teach vacuum_one_database() to reuse query results.

  1. vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-01-24T15:44:46Z

    On Mon, Jan 06, 2025 at 03:27:18PM -0600, Nathan Bossart wrote:
> On Mon, Dec 30, 2024 at 03:45:03PM -0500, Bruce Momjian wrote:
>> On Mon, Dec 30, 2024 at 12:02:47PM -0800, Jeff Davis wrote:
>>> I suggest that we make a new thread about the vacuumdb changes and
>>> focus this thread and patch series on the pg_dump changes (and minor
>>> flag adjustments to pg_upgrade).
>>> 
>>> Unless you think that the pg_dump changes should block on the vacuumdb
>>> changes? In which case please let me know because the pg_dump changes
>>> are otherwise close to commit.
>> 
>> I think that is a good idea.  I don't see vacuumdb blocking this.
> 
> +1, I've been reviewing the vacuumdb portion and am planning to start a new
> thread in the near future.  IIUC the bulk of the vacuumdb changes are
> relatively noncontroversial, we just haven't reached consensus on the user
> interface.

As promised, I'm starting a new thread for this.  The original thread [0]
has some preliminary discussion about the subject.

As you may be aware, there is an ongoing effort to carry over statistics
during pg_upgrade.  Today, we encourage users to use vacuumdb to run
ANALYZE on all relations after upgrading.  There's even a special
--analyze-in-stages option that fast-tracks an initial set of minimal
statistics for this use-case.  Once the statistics are carried over by
pg_upgrade, there will be little need to do this, except for perhaps
extended statistics if they aren't carried over.  But there are patches in
flight for that, too [1].

This thread is dedicated to figuring out what, if anything, to do about
vacuumdb.  I see the following general categories of options:

* Do nothing.  Other than updating our recommended guidance for
  post-upgrade analyzing, we'd leave vacuumdb alone.  While this is
  certainly a simple option, it has a couple of key drawbacks.  For one,
  anyone who doesn't see the new vacuumdb guidance may continue to do
  unnecessary post-upgrade analyzes.  Also, if we don't get the extended
  statistics piece completed for v18, users will have to manually construct
  ANALYZE commands for those to run post-upgrade.

* Add a breaking change so that users are forced to fix any outdated
  post-upgrade scripts.  This is what the attached patches do.  In short,
  they add a required parameter to --analyze-in-stages that can be set to
  either "all" or "missing".  The new "missing" mode generates ANALYZE
  commands for relations that are missing statistics, while the "all" mode
  does the same thing that --analyze-in-stages does today.  While the
  "missing" mode might be useful outside of upgrade cases, we could also
  recommend it as a post-upgrade step if the extended statistics work
  doesn't get committed for v18.

* Add a new option that will make it easy to ANALYZE any relations that are
  missing statistics, but don't make any breaking changes to existing
  post-upgrade scripts.  This option isn't really strictly necessary if we
  get the extended statistics parts committed, but it could be a nice
  feature, anyway.

I chose the second approach because it had the most support in the other
thread, but I definitely wouldn't characterize it as a consensus.  0001
simply refactors the main catalog query to its own function so that its
results can be reused in later stages of --analyze-in-stages.  This might
require a bit more memory and make --analyze-in-stages less responsive to
concurrent changes, but it wasn't all that responsive to begin with.  0002
adds the new "missing" mode functionality.  Note that it regenerates all
statistics for a relation if any applicable statistics types are missing.
It's not clear whether we can or should do any better than that.  Corey and
I put a lot of effort into the catalog query changes, and we think we've
covered everything, but we would of course appreciate some review on that
part.

BTW as long as the basic "missing" mode idea seems reasonable, it's easy
enough to adjust the user interface to whatever we want, and I'm happy to
do so as needed.

Finally, I think another open question is whether any of this should apply
to --analyze and/or --analyze-only.  We do recommend the latter as a
post-upgrade step in our pg_upgrade documentation, and I could see the
"missing" mode being useful on its own for these modes, too.

Thoughts?

[0] https://postgr.es/m/CADkLM%3DfR7TwH0cLREQkf5_%3DKLcOYVxJw0Km0i5MpaWeuDwVo6g%40mail.gmail.com
[1] https://postgr.es/m/CADkLM%3Ddpz3KFnqP-dgJ-zvRvtjsa8UZv8wDAQdqho%3DqN3kX0Zg%40mail.gmail.com

-- 
nathan

  2. Re: vacuumdb changes for stats import/export

    Corey Huinker <corey.huinker@gmail.com> — 2025-01-24T16:19:12Z

    >
> Thoughts?
>

I don't have anything to add to what Nathan said, but thought I should say
so since this thread was broken off from my earlier thread. Eagerly
awaiting feedback.

  3. Re: vacuumdb changes for stats import/export

    Masahiko Sawada <sawada.mshk@gmail.com> — 2025-01-24T21:25:31Z

    On Fri, Jan 24, 2025 at 7:44 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Mon, Jan 06, 2025 at 03:27:18PM -0600, Nathan Bossart wrote:
> > On Mon, Dec 30, 2024 at 03:45:03PM -0500, Bruce Momjian wrote:
> >> On Mon, Dec 30, 2024 at 12:02:47PM -0800, Jeff Davis wrote:
> >>> I suggest that we make a new thread about the vacuumdb changes and
> >>> focus this thread and patch series on the pg_dump changes (and minor
> >>> flag adjustments to pg_upgrade).
> >>>
> >>> Unless you think that the pg_dump changes should block on the vacuumdb
> >>> changes? In which case please let me know because the pg_dump changes
> >>> are otherwise close to commit.
> >>
> >> I think that is a good idea.  I don't see vacuumdb blocking this.
> >
> > +1, I've been reviewing the vacuumdb portion and am planning to start a new
> > thread in the near future.  IIUC the bulk of the vacuumdb changes are
> > relatively noncontroversial, we just haven't reached consensus on the user
> > interface.
>
> As promised, I'm starting a new thread for this.  The original thread [0]
> has some preliminary discussion about the subject.
>
> As you may be aware, there is an ongoing effort to carry over statistics
> during pg_upgrade.  Today, we encourage users to use vacuumdb to run
> ANALYZE on all relations after upgrading.  There's even a special
> --analyze-in-stages option that fast-tracks an initial set of minimal
> statistics for this use-case.  Once the statistics are carried over by
> pg_upgrade, there will be little need to do this, except for perhaps
> extended statistics if they aren't carried over.  But there are patches in
> flight for that, too [1].
>
> This thread is dedicated to figuring out what, if anything, to do about
> vacuumdb.  I see the following general categories of options:
>
> * Do nothing.  Other than updating our recommended guidance for
>   post-upgrade analyzing, we'd leave vacuumdb alone.  While this is
>   certainly a simple option, it has a couple of key drawbacks.  For one,
>   anyone who doesn't see the new vacuumdb guidance may continue to do
>   unnecessary post-upgrade analyzes.  Also, if we don't get the extended
>   statistics piece completed for v18, users will have to manually construct
>   ANALYZE commands for those to run post-upgrade.
>
> * Add a breaking change so that users are forced to fix any outdated
>   post-upgrade scripts.  This is what the attached patches do.  In short,
>   they add a required parameter to --analyze-in-stages that can be set to
>   either "all" or "missing".  The new "missing" mode generates ANALYZE
>   commands for relations that are missing statistics, while the "all" mode
>   does the same thing that --analyze-in-stages does today.  While the
>   "missing" mode might be useful outside of upgrade cases, we could also
>   recommend it as a post-upgrade step if the extended statistics work
>   doesn't get committed for v18.
>
> * Add a new option that will make it easy to ANALYZE any relations that are
>   missing statistics, but don't make any breaking changes to existing
>   post-upgrade scripts.  This option isn't really strictly necessary if we
>   get the extended statistics parts committed, but it could be a nice
>   feature, anyway.
>
> I chose the second approach because it had the most support in the other
> thread, but I definitely wouldn't characterize it as a consensus.  0001
> simply refactors the main catalog query to its own function so that its
> results can be reused in later stages of --analyze-in-stages.  This might
> require a bit more memory and make --analyze-in-stages less responsive to
> concurrent changes, but it wasn't all that responsive to begin with.  0002
> adds the new "missing" mode functionality.  Note that it regenerates all
> statistics for a relation if any applicable statistics types are missing.
> It's not clear whether we can or should do any better than that.  Corey and
> I put a lot of effort into the catalog query changes, and we think we've
> covered everything, but we would of course appreciate some review on that
> part.
>
> BTW as long as the basic "missing" mode idea seems reasonable, it's easy
> enough to adjust the user interface to whatever we want, and I'm happy to
> do so as needed.
>
> Finally, I think another open question is whether any of this should apply
> to --analyze and/or --analyze-only.  We do recommend the latter as a
> post-upgrade step in our pg_upgrade documentation, and I could see the
> "missing" mode being useful on its own for these modes, too.
>
>Thoughts?

I've not closely reviewed the patches yet but I find that the second
approach is reasonable to me. IIUC if the extended statistics
import/export work gets committed for v18, we don't need a new option
for post-upgrade analyze. But as you mentioned, these new modes seem
useful for other use cases too. Given that it could be used outside of
post-upgrading, I think it would make more sense to apply the "all"
and "missing" modes to --analyze and --analyze-only options too.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

  4. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-02-04T21:43:57Z

    I had the opportunity to bring this patch set up for discussion at the
developer meeting at FOSDEM PGDay last week [0].  There seemed to be a
strong consensus that the idea of a "missing only" mode for vacuumdb's
analyze options was useful (especially so if the extended stats piece of
the stats import/export project doesn't make it into v18), but that we
shouldn't change the default behavior of the existing options.  Given that,
I have modified the patch set to instead introduce a --missing-only option
that can be used in conjuction with --analyze-only and --analyze-in-stages.
The underlying implementation is the same as in v1 of the patch set, except
for the following changes:

* I've modified the extended stats part of the query to also check for
  pg_statistic_ext.stxstattarget IS DISTINCT FROM 0.

* I've added a new clause to check for extended statistics on tables with
  inheritance children, i.e., those with pg_statistic_ext_data.stxdinherit
  set to true.

* I've added a server version check that disallows --missing-only on
  servers older than v15.  The catalog query would need some adjustments to
  work on older versions, but that didn't seem critically important.  We
  could always revisit this in the future.

[0] https://2025.fosdempgday.org/devmeeting

-- 
nathan

  5. Re: vacuumdb changes for stats import/export

    John Naylor <johncnaylorls@gmail.com> — 2025-02-27T09:36:04Z

    On Wed, Feb 5, 2025 at 4:44 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
> [v2]

I started looking just at 0001, and it seems like a fairly
straightforward rearrangement. I found this comment quite hard to
read:

+ * 'found_objs' should be a fully qualified list of objects to process, as
+ * returned by a previous call to vacuum_one_database().  If *found_objs is
+ * NULL, it is set to the results of the catalog query discussed below.  If
+ * found_objs is NULL, the results of the catalog query are not returned.
+ *
+ * If *found_objs is NULL, this function performs a catalog query to retrieve
+ * the list of tables to process.  When 'objects' is NULL, all tables in the

I had to read it several times before I noticed the difference between
"* found_objs" and "*found_objs". Maybe some extra spacing and breaks
would help, or other reorganization.

-- 
John Naylor
Amazon Web Services

  6. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-02-28T20:42:11Z

    On Thu, Feb 27, 2025 at 04:36:04PM +0700, John Naylor wrote:
> On Wed, Feb 5, 2025 at 4:44 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> [v2]
> 
> I started looking just at 0001, and it seems like a fairly
> straightforward rearrangement.

Thanks for taking a look.

> I found this comment quite hard to read:
> 
> + * 'found_objs' should be a fully qualified list of objects to process, as
> + * returned by a previous call to vacuum_one_database().  If *found_objs is
> + * NULL, it is set to the results of the catalog query discussed below.  If
> + * found_objs is NULL, the results of the catalog query are not returned.
> + *
> + * If *found_objs is NULL, this function performs a catalog query to retrieve
> + * the list of tables to process.  When 'objects' is NULL, all tables in the
> 
> I had to read it several times before I noticed the difference between
> "* found_objs" and "*found_objs". Maybe some extra spacing and breaks
> would help, or other reorganization.

Yeah, it's pretty atrocious.  I think the main problem is that the
interface is just too complicated, so I'll take a step back and see if I
can make it more understandable to humans.  In the meantime, here's an
attempt at adjusting the comment:

 * found_objs is a double pointer to a fully qualified list of objects to
 * process, as returned by a previous call to vacuum_one_database().  If
 * *found_objs (the once-dereferenced double pointer) is NULL, it is set to the
 * results of the catalog query discussed below.  If found_objs (the double
 * pointer) is NULL, the results of the catalog query are not returned.
 *
 * If *found_objs (the once-dereferenced double pointer) is NULL, this function
 * performs a catalog query to retrieve the list of tables to process.  When
 * "objects" is NULL, all tables in the database are processed.  Otherwise, the
 * catalog query performs a lookup for the objects listed in "objects".

-- 
nathan

  7. Re: vacuumdb changes for stats import/export

    John Naylor <johncnaylorls@gmail.com> — 2025-03-03T10:58:43Z

    On Sat, Mar 1, 2025 at 3:42 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Thu, Feb 27, 2025 at 04:36:04PM +0700, John Naylor wrote:

> > I had to read it several times before I noticed the difference between
> > "* found_objs" and "*found_objs". Maybe some extra spacing and breaks
> > would help, or other reorganization.
>
> Yeah, it's pretty atrocious.  I think the main problem is that the
> interface is just too complicated, so I'll take a step back and see if I
> can make it more understandable to humans.

The interface is awkward, but on the other hand only a small part has
to really know about it. It's worth trying to make it more readable if
you can.

> In the meantime, here's an
> attempt at adjusting the comment:

That's better, and if we end up with this interface, we'll want quotes
around the names so the eye can tell where the "*" belong.

-- 
John Naylor
Amazon Web Services

  8. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-03-03T16:21:37Z

    On Mon, Mar 03, 2025 at 05:58:43PM +0700, John Naylor wrote:
> On Sat, Mar 1, 2025 at 3:42 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> On Thu, Feb 27, 2025 at 04:36:04PM +0700, John Naylor wrote:
>> > I had to read it several times before I noticed the difference between
>> > "* found_objs" and "*found_objs". Maybe some extra spacing and breaks
>> > would help, or other reorganization.
>>
>> Yeah, it's pretty atrocious.  I think the main problem is that the
>> interface is just too complicated, so I'll take a step back and see if I
>> can make it more understandable to humans.
> 
> The interface is awkward, but on the other hand only a small part has
> to really know about it. It's worth trying to make it more readable if
> you can.

True.  One small thing we could do is to require "found_objs" (the double
pointer) to always be non-NULL, but that just compels some callers to
provide otherwise-unused variables.  I've left the interface alone for now.

>> In the meantime, here's an attempt at adjusting the comment:
> 
> That's better, and if we end up with this interface, we'll want quotes
> around the names so the eye can tell where the "*" belong.

I did that in v3.  I also tried to break up this comment into bullet points
for better separation and logical flow.

-- 
nathan

  9. Re: vacuumdb changes for stats import/export

    John Naylor <johncnaylorls@gmail.com> — 2025-03-04T06:05:17Z

    On Mon, Mar 3, 2025 at 11:21 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Mon, Mar 03, 2025 at 05:58:43PM +0700, John Naylor wrote:
> True.  One small thing we could do is to require "found_objs" (the double
> pointer) to always be non-NULL, but that just compels some callers to
> provide otherwise-unused variables.  I've left the interface alone for now.

One thing to consider is that a pointer named "dummy" is self-documenting.

> > That's better, and if we end up with this interface, we'll want quotes
> > around the names so the eye can tell where the "*" belong.
>
> I did that in v3.  I also tried to break up this comment into bullet points
> for better separation and logical flow.

That's much easier to follow, thanks.

-- 
John Naylor
Amazon Web Services

  10. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-03-04T17:13:23Z

    On Tue, Mar 04, 2025 at 01:05:17PM +0700, John Naylor wrote:
> On Mon, Mar 3, 2025 at 11:21 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> I did that in v3.  I also tried to break up this comment into bullet points
>> for better separation and logical flow.
> 
> That's much easier to follow, thanks.

Thanks for looking.  I'll probably commit 0001 sooner than later so that we
can focus our attention on 0002.

-- 
nathan

  11. Re: vacuumdb changes for stats import/export

    John Naylor <johncnaylorls@gmail.com> — 2025-03-06T11:30:59Z

    On Wed, Mar 5, 2025 at 12:13 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Tue, Mar 04, 2025 at 01:05:17PM +0700, John Naylor wrote:
> > On Mon, Mar 3, 2025 at 11:21 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> >> I did that in v3.  I also tried to break up this comment into bullet points
> >> for better separation and logical flow.
> >
> > That's much easier to follow, thanks.
>
> Thanks for looking.  I'll probably commit 0001 sooner than later so that we
> can focus our attention on 0002.

+1

On 0002:

+        This option can only be used in conjunction with
+        <option>--analyze-only</option> and
<option>--analyze-in-stages</option>.

+ /* Prohibit --missing-only without --analyze-only or --analyze-in-stages */
+ if (vacopts.missing_only && !vacopts.analyze_only)
+ pg_fatal("cannot use the \"%s\" option without \"%s\" or \"%s\"",
+ "missing-only", "analyze-only", "analyze-in-stages");

The first is slightly ambiguous, so maybe "or" is better throughout.

+ " CROSS JOIN LATERAL (SELECT c.relkind IN ('p', 'I')) as p (inherited)\n"

Looking elsewhere in this file, I think we prefer something like
"(c.relkind OPERATOR(pg_catalog.=) ANY (array["
  CppAsString2(RELKIND_PARTITIONED_TABLE) ", "
  CppAsString2(RELKIND_PARTITIONED_INDEX) "]) as p (inherited)\n"

+ " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_statistic s\n"
+ " WHERE s.starelid OPERATOR(pg_catalog.=) a.attrelid\n"
+ " AND s.staattnum OPERATOR(pg_catalog.=) a.attnum\n"
+ " AND s.stainherit OPERATOR(pg_catalog.=) p.inherited))\n");

IIUC correctly, pg_statistic doesn't store stats on itself, so this
causes the query result to always contain pg_statistic -- does that
get removed elsewhere?

--
John Naylor
Amazon Web Services

  12. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-03-06T21:47:06Z

    On Thu, Mar 06, 2025 at 06:30:59PM +0700, John Naylor wrote:
> +        This option can only be used in conjunction with
> +        <option>--analyze-only</option> and
> <option>--analyze-in-stages</option>.
> 
> + /* Prohibit --missing-only without --analyze-only or --analyze-in-stages */
> + if (vacopts.missing_only && !vacopts.analyze_only)
> + pg_fatal("cannot use the \"%s\" option without \"%s\" or \"%s\"",
> + "missing-only", "analyze-only", "analyze-in-stages");
> 
> The first is slightly ambiguous, so maybe "or" is better throughout.

Agreed.

> + " CROSS JOIN LATERAL (SELECT c.relkind IN ('p', 'I')) as p (inherited)\n"
> 
> Looking elsewhere in this file, I think we prefer something like
> "(c.relkind OPERATOR(pg_catalog.=) ANY (array["
>   CppAsString2(RELKIND_PARTITIONED_TABLE) ", "
>   CppAsString2(RELKIND_PARTITIONED_INDEX) "]) as p (inherited)\n"

Fixed.

> + " AND NOT EXISTS (SELECT NULL FROM pg_catalog.pg_statistic s\n"
> + " WHERE s.starelid OPERATOR(pg_catalog.=) a.attrelid\n"
> + " AND s.staattnum OPERATOR(pg_catalog.=) a.attnum\n"
> + " AND s.stainherit OPERATOR(pg_catalog.=) p.inherited))\n");
> 
> IIUC correctly, pg_statistic doesn't store stats on itself, so this
> causes the query result to always contain pg_statistic -- does that
> get removed elsewhere?

Good catch.  I think the current behavior is to call ANALYZE on
pg_statistic, too, but that should be mostly harmless (analyze_rel()
refuses to process it).  I suppose we could try to avoid returning
pg_statistic from the catalog query, but we don't bother doing that for any
other vacuumdb modes, so I'm tempted to leave it alone.

-- 
nathan

  13. Re: vacuumdb changes for stats import/export

    John Naylor <johncnaylorls@gmail.com> — 2025-03-10T05:35:22Z

    On Fri, Mar 7, 2025 at 4:47 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Thu, Mar 06, 2025 at 06:30:59PM +0700, John Naylor wrote:

> > IIUC correctly, pg_statistic doesn't store stats on itself, so this
> > causes the query result to always contain pg_statistic -- does that
> > get removed elsewhere?
>
> Good catch.  I think the current behavior is to call ANALYZE on
> pg_statistic, too, but that should be mostly harmless (analyze_rel()
> refuses to process it).  I suppose we could try to avoid returning
> pg_statistic from the catalog query, but we don't bother doing that for any
> other vacuumdb modes, so I'm tempted to leave it alone.

Okay, thanks for confirming. I have no further comments.

-- 
John Naylor
Amazon Web Services

  14. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-03-10T15:08:49Z

    On Mon, Mar 10, 2025 at 12:35:22PM +0700, John Naylor wrote:
> I have no further comments.

Thanks.  I'll give this a little more time for review before committing.

We'll still need to update the recommendation in pg_upgrade's
documentation.  I'm going to keep that separate because the stats
import/export work is still settling.

-- 
nathan

  15. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-03-11T17:00:01Z

    On Mon, Mar 10, 2025 at 10:08:49AM -0500, Nathan Bossart wrote:
> On Mon, Mar 10, 2025 at 12:35:22PM +0700, John Naylor wrote:
>> I have no further comments.
> 
> Thanks.  I'll give this a little more time for review before committing.

I realized that we could limit the catalog query reuse to only when
--missing-only is specified, so I've updated 0001 and 0002 accordingly.
This avoids changing any existing behavior.

> We'll still need to update the recommendation in pg_upgrade's
> documentation.  I'm going to keep that separate because the stats
> import/export work is still settling.

0003 is a first attempt at this.  Unless I am missing something, there's
really not much to update.

-- 
nathan

  16. Re: vacuumdb changes for stats import/export

    John Naylor <johncnaylorls@gmail.com> — 2025-03-12T06:56:04Z

    On Wed, Mar 12, 2025 at 12:00 AM Nathan Bossart
<nathandbossart@gmail.com> wrote:
>
> On Mon, Mar 10, 2025 at 10:08:49AM -0500, Nathan Bossart wrote:
> > On Mon, Mar 10, 2025 at 12:35:22PM +0700, John Naylor wrote:
> >> I have no further comments.
> >
> > Thanks.  I'll give this a little more time for review before committing.
>
> I realized that we could limit the catalog query reuse to only when
> --missing-only is specified, so I've updated 0001 and 0002 accordingly.
> This avoids changing any existing behavior.
>
> > We'll still need to update the recommendation in pg_upgrade's
> > documentation.  I'm going to keep that separate because the stats
> > import/export work is still settling.
>
> 0003 is a first attempt at this.  Unless I am missing something, there's
> really not much to update.

The change here seems fine. My only quibble is that this sentence now
seems out of place: "Option --analyze-in-stages can be used to
generate minimal statistics quickly." I'm thinking we should make a
clearer separation for with and without the  --no-statistics option
specified.


--
John Naylor
Amazon Web Services

  17. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-03-12T19:34:06Z

    On Wed, Mar 12, 2025 at 01:56:04PM +0700, John Naylor wrote:
> The change here seems fine. My only quibble is that this sentence now
> seems out of place: "Option --analyze-in-stages can be used to
> generate minimal statistics quickly." I'm thinking we should make a
> clearer separation for with and without the  --no-statistics option
> specified.

I think the recommendation stays the same regardless of whether
--no-statistics was used.  --missing-only should do the right think either
way.  But I do agree that this paragraph feels a bit haphazard.  I modified
it to call out the full command for both --analyze-only and
--analyze-in-stages, and I moved the note about --jobs to its own sentence
and made it clear that it is applicable to both of the suggested vacuumdb
commands.

-- 
nathan

  18. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-03-14T19:05:30Z

    Out of curiosity, I generated many relations with the following command
(stolen from [0]):

do $$
begin
for i in 1..100000 loop
  execute format('create table t%s (f1 int unique, f2 int unique);', i);
  execute format('insert into t%s select x, x from generate_series(1,1000) x',
                 i);
  if i % 100 = 0 then commit; end if;
end loop;
end
$$;

And then I ran a database-wide ANALYZE.  Without --missing-only, vacuumdb's
catalog query took 65 ms.  With --missing-only, it took 735 ms.  While
that's a big jump, this query will only run once for a given vacuumdb, and
--missing-only is likely to save a lot of time elsewhere.

If no feedback or objections materialize, I'm planning to commit these
early next week.

[0] https://postgr.es/m/3612876.1689443232%40sss.pgh.pa.us

-- 
nathan

  19. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-03-18T02:17:31Z

    On Fri, Mar 14, 2025 at 02:05:30PM -0500, Nathan Bossart wrote:
> If no feedback or objections materialize, I'm planning to commit these
> early next week.

While preparing this for commit, I noticed that the expression index part
of the query was disregarding attstattarget.  To fix, I've modified that
part to look at the index's pg_attribute entries.

-- 
nathan

  20. Re: vacuumdb changes for stats import/export

    Corey Huinker <corey.huinker@gmail.com> — 2025-03-18T02:51:27Z

    >
>
> While preparing this for commit, I noticed that the expression index part
> of the query was disregarding attstattarget.  To fix, I've modified that
> part to look at the index's pg_attribute entries.
>

+1, should have been there all along.

  21. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-03-18T21:37:52Z

    Committed with the following small changes:

* I renamed the new option to --missing-stats-only, which I felt was more
  descriptive.

* I moved the new tests to the main vacuumdb test file and interspersed
  some --analyze-only uses.

* I fixed a couple of other things in the new parts of the catalog query
  that were not fully qualified.

-- 
nathan

  22. Re: vacuumdb changes for stats import/export

    Frédéric Yhuel <frederic.yhuel@dalibo.com> — 2025-07-23T12:45:10Z

    
On 3/18/25 22:37, Nathan Bossart wrote:
> Committed with the following small changes:

Hi, I don't really understand this sentence in 
doc/src/sgml/ref/vacuumdb.sgml:

 > This option prevents vacuumdb from deleting existing statistics so 
that the query optimizer's choices do not become transiently worse.

I thought that the point was to avoid unnecessary post-upgrade analyzes?

  23. Re: vacuumdb changes for stats import/export

    Bruce Momjian <bruce@momjian.us> — 2025-07-26T14:22:49Z

    On Wed, Jul 23, 2025 at 02:45:10PM +0200, Frédéric Yhuel wrote:
> 
> 
> On 3/18/25 22:37, Nathan Bossart wrote:
> > Committed with the following small changes:
> 
> Hi, I don't really understand this sentence in
> doc/src/sgml/ref/vacuumdb.sgml:
> 
> > This option prevents vacuumdb from deleting existing statistics so that
> the query optimizer's choices do not become transiently worse.
> 
> I thought that the point was to avoid unnecessary post-upgrade analyzes?

So, the full paragraph is:

+        Only analyze relations that are missing statistics for a column, index
+        expression, or extended statistics object.  This option prevents
+        <application>vacuumdb</application> from deleting existing statistics
+        so that the query optimizer's choices do not become transiently worse.

What it is trying to say is that if you run vacuumedb without this
option, not only will it analyze all tables, including ones that already
have statistics, but will drop statistics on this tables that already
have statistics for a brief period while it installs new statistics. 
During that period, the optimizer will not have any statistics for the
table.  Is there a clearer way to state this?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.

  24. Re: vacuumdb changes for stats import/export

    David G. Johnston <david.g.johnston@gmail.com> — 2025-07-26T14:47:47Z

    On Sat, Jul 26, 2025, 07:23 Bruce Momjian <bruce@momjian.us> wrote:

> On Wed, Jul 23, 2025 at 02:45:10PM +0200, Frédéric Yhuel wrote:
> >
> >
> > On 3/18/25 22:37, Nathan Bossart wrote:
> > > Committed with the following small changes:
> >
> > Hi, I don't really understand this sentence in
> > doc/src/sgml/ref/vacuumdb.sgml:
> >
> > > This option prevents vacuumdb from deleting existing statistics so that
> > the query optimizer's choices do not become transiently worse.
> >
> > I thought that the point was to avoid unnecessary post-upgrade analyzes?
>
> So, the full paragraph is:
>
> +        Only analyze relations that are missing statistics for a column,
> index
> +        expression, or extended statistics object.  This option prevents
> +        <application>vacuumdb</application> from deleting existing
> statistics
> +        so that the query optimizer's choices do not become transiently
> worse.
>
> What it is trying to say is that if you run vacuumedb without this
> option, not only will it analyze all tables, including ones that already
> have statistics, but will drop statistics on this tables that already
> have statistics for a brief period while it installs new statistics.
> During that period, the optimizer will not have any statistics for the
> table.  Is there a clearer way to state this?
>

Statistics are transactional.  Without this option specified are we really
removing them and commiting prior to computing and saving new ones?  And we
are opposed to just changing this behavior and instead prefer to add an
option that at first glance seems like everyone should use?

"If not specified the system will analyze all statistics-capable objects in
alphabetical order.  Specifying this option then limits the result to only
those objects that do not already have statistics.". That may not be how
the feature strictly behaves but that would seem to be all one would expect
it to do.

David J.

  25. Re: vacuumdb changes for stats import/export

    Bruce Momjian <bruce@momjian.us> — 2025-07-26T16:06:52Z

    On Sat, Jul 26, 2025 at 07:47:47AM -0700, David G. Johnston wrote:
> On Sat, Jul 26, 2025, 07:23 Bruce Momjian <bruce@momjian.us> wrote:
> 
>     On Wed, Jul 23, 2025 at 02:45:10PM +0200, Frédéric Yhuel wrote:
>     >
>     >
>     > On 3/18/25 22:37, Nathan Bossart wrote:
>     > > Committed with the following small changes:
>     >
>     > Hi, I don't really understand this sentence in
>     > doc/src/sgml/ref/vacuumdb.sgml:
>     >
>     > > This option prevents vacuumdb from deleting existing statistics so that
>     > the query optimizer's choices do not become transiently worse.
>     >
>     > I thought that the point was to avoid unnecessary post-upgrade analyzes?
> 
>     So, the full paragraph is:
> 
>     +        Only analyze relations that are missing statistics for a column,
>     index
>     +        expression, or extended statistics object.  This option prevents
>     +        <application>vacuumdb</application> from deleting existing
>     statistics
>     +        so that the query optimizer's choices do not become transiently
>     worse.
> 
>     What it is trying to say is that if you run vacuumedb without this
>     option, not only will it analyze all tables, including ones that already
>     have statistics, but will drop statistics on this tables that already
>     have statistics for a brief period while it installs new statistics.
>     During that period, the optimizer will not have any statistics for the
>     table.  Is there a clearer way to state this?
> 
> Statistics are transactional.  Without this option specified are we really
> removing them and commiting prior to computing and saving new ones?  And we are
> opposed to just changing this behavior and instead prefer to add an option that
> at first glance seems like everyone should use?

Yes, I thought it was transactional too, but the doc patch suggests is
isn't, so maybe I am wrong.

> "If not specified the system will analyze all statistics-capable objects in
> alphabetical order.  Specifying this option then limits the result to only
> those objects that do not already have statistics.". That may not be how the
> feature strictly behaves but that would seem to be all one would expect it to
> do.

Yes, I would prefer the simpler text, if it is accurate.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.

  26. Re: vacuumdb changes for stats import/export

    Corey Huinker <corey.huinker@gmail.com> — 2025-07-26T19:38:09Z

    >
>
> >
> > Statistics are transactional.  Without this option specified are we
> really
> > removing them and commiting prior to computing and saving new ones?  And
> we are
> > opposed to just changing this behavior and instead prefer to add an
> option that
> > at first glance seems like everyone should use?
>
> Yes, I thought it was transactional too, but the doc patch suggests is
> isn't, so maybe I am wrong.
>

It is transactional, mostly.

The attribute stats for the table being analyzed and all attribute stats
for the dependent indexes that have at least one expression column, plus
extended stats objects on that table, will be replaced in one atomic
operation. The old stats were there, commit happens, now the new stats are
there.

The relation stats (pg_class) happen in-place, non-transactionally. So if
you had an analyze that got canceled, and some of the relstats had been
updated on the table or indexes, those would stay as-is.

Having said all that, these are very nitpick details that may not need to
be in our documentation.


>
> > "If not specified the system will analyze all statistics-capable objects
> in
> > alphabetical order.  Specifying this option then limits the result to
> only
> > those objects that do not already have statistics.". That may not be how
> the
> > feature strictly behaves but that would seem to be all one would expect
> it to
> > do.
>

/me dons Hat of Pedantry (+2 against simple explanations)

"Specifying this option then limits the result to only tables that are
missing an attribute statistic, have an index that is missing an attribute
statistic, or have an extended statistics object that is itself missing
object statistics".

/me removes hat

"Specifying this option then limits the result to only those tables that do
not already have complete statistics."

The above phrase is asking the word "complete" to do a lot of heavy
lifting, covering the dependent indexes and extended statistics objects.
Specifying "table" makes it clear that any deficit in statistics results in
the table getting analyzed, not just the single index or extended object.

  27. Re: vacuumdb changes for stats import/export

    Frédéric Yhuel <frederic.yhuel@dalibo.com> — 2025-07-27T10:46:44Z

    
On 7/26/25 21:38, Corey Huinker wrote:
> 
> 
> It is transactional, mostly.
> 
> The attribute stats for the table being analyzed and all attribute stats 
> for the dependent indexes that have at least one expression column, plus 
> extended stats objects on that table, will be replaced in one atomic 
> operation. The old stats were there, commit happens, now the new stats 
> are there.
> 
> The relation stats (pg_class) happen in-place, non-transactionally. So 
> if you had an analyze that got canceled, and some of the relstats had 
> been updated on the table or indexes, those would stay as-is.
> 

Then it seems very unlikely that the query optimizer's choices become 
transiently worse because of that, doesn't it? and this shouldn't be 
used to justify this option, IMHO.

How about this? "This option is primarily intended to be used in 
post-upgrade scripts to quickly generate minimal optimizer statistics"

> Having said all that, these are very nitpick details that may not need 
> to be in our documentation.
> 

+1

I've always supposed that the statistics were completely transactional, 
I think most users do, and the difference with reality seems small enough.

> 
> 
> 
> /me dons Hat of Pedantry (+2 against simple explanations)
> 
> "Specifying this option then limits the result to only tables that are 
> missing an attribute statistic, have an index that is missing an 
> attribute statistic, or have an extended statistics object that is 
> itself missing object statistics".
> 
> /me removes hat
> 
> "Specifying this option then limits the result to only those tables that 
> do not already have complete statistics."
> 
> The above phrase is asking the word "complete" to do a lot of heavy 
> lifting, covering the dependent indexes and extended statistics objects. 
> Specifying "table" makes it clear that any deficit in statistics results 
> in the table getting analyzed, not just the single index or extended object.

FWIW I much prefer the last, simple phrase.

  28. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-07-28T14:47:22Z

    On Sun, Jul 27, 2025 at 12:46:44PM +0200, Frédéric Yhuel wrote:
> Then it seems very unlikely that the query optimizer's choices become
> transiently worse because of that, doesn't it? and this shouldn't be used to
> justify this option, IMHO.

I can't remember who wrote this line, but it was borrowed from the
--analyze-in-stages description.  The point is that if you use
--analyze-in-stages without --missing-stats-only, there will be a period
where existing statistics will be replaced with ones generated with lower
statistics targets.  Obviously, this wording isn't clear enough.  We might
need to either remove that sentence or add "When used in conjunction with
--analyze-in-stages..."

-- 
nathan

  29. Re: vacuumdb changes for stats import/export

    Frédéric Yhuel <frederic.yhuel@dalibo.com> — 2025-07-28T15:09:15Z

    
On 7/28/25 16:47, Nathan Bossart wrote:
> I can't remember who wrote this line, but it was borrowed from the
> --analyze-in-stages description.  The point is that if you use
> --analyze-in-stages without --missing-stats-only, there will be a period
> where existing statistics will be replaced with ones generated with lower
> statistics targets.

Aha, it makes sense now, thank you!

> Obviously, this wording isn't clear enough.  We might
> need to either remove that sentence or add "When used in conjunction with
> --analyze-in-stages..."

I vote for the second option.

  30. Re: vacuumdb changes for stats import/export

    David G. Johnston <david.g.johnston@gmail.com> — 2025-07-28T16:22:29Z

    On Monday, July 28, 2025, Frédéric Yhuel <frederic.yhuel@dalibo.com> wrote:

>
>
> On 7/28/25 16:47, Nathan Bossart wrote:
>
>> I can't remember who wrote this line, but it was borrowed from the
>> --analyze-in-stages description.  The point is that if you use
>> --analyze-in-stages without --missing-stats-only, there will be a period
>> where existing statistics will be replaced with ones generated with lower
>> statistics targets.
>>
>
> Aha, it makes sense now, thank you!
>
> Obviously, this wording isn't clear enough.  We might
>> need to either remove that sentence or add "When used in conjunction with
>> --analyze-in-stages..."
>>
>
> I vote for the second option.
>
>
Makes sense.  This does beg the question - what happens if a column is left
with a lower statistics target than what would be applied during an
analyze, but one is present?  I don’t see where the statistics target is
saved anywhere.  Can we start recording that piece of data and teach
analyze in stages to just never go backwards - reporting any it had to skip
to adhere to that rule.  Seems like a better policy than missing-only.

David J.

  31. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-07-28T18:34:11Z

    On Mon, Jul 28, 2025 at 09:22:29AM -0700, David G. Johnston wrote:
> Makes sense.  This does beg the question - what happens if a column is left
> with a lower statistics target than what would be applied during an
> analyze, but one is present?  I don´t see where the statistics target is
> saved anywhere.  Can we start recording that piece of data and teach
> analyze in stages to just never go backwards - reporting any it had to skip
> to adhere to that rule.  Seems like a better policy than missing-only.

That would involve changing the behavior of an existing option, which has
thus far been strongly opposed [0], for what strikes me as a reasonably
uncommon situation.  Plus, --missing-stats-only may be useful in other
situations.

[0] https://postgr.es/m/Z6KKHX9PZkB19lAK%40nathan

-- 
nathan

  32. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-07-28T18:46:34Z

    On Mon, Jul 28, 2025 at 09:22:29AM -0700, David G. Johnston wrote:
> On Monday, July 28, 2025, Frédéric Yhuel <frederic.yhuel@dalibo.com> wrote:
>> On 7/28/25 16:47, Nathan Bossart wrote:
>>> Obviously, this wording isn't clear enough.  We might need to either
>>> remove that sentence or add "When used in conjunction with
>>> --analyze-in-stages..."
>>
>> I vote for the second option.
>
> Makes sense. 

It seems like there is some support for adding "When used in conjunction
with --analyze in stages..." to the beginning of the sentence.  I'll give
it another day or two for any further discussion before committing.

-- 
nathan

  33. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-07-30T17:21:08Z

    On Mon, Jul 28, 2025 at 01:46:34PM -0500, Nathan Bossart wrote:
> On Mon, Jul 28, 2025 at 09:22:29AM -0700, David G. Johnston wrote:
>> On Monday, July 28, 2025, Frédéric Yhuel <frederic.yhuel@dalibo.com> wrote:
>>> On 7/28/25 16:47, Nathan Bossart wrote:
>>>> Obviously, this wording isn't clear enough.  We might need to either
>>>> remove that sentence or add "When used in conjunction with
>>>> --analyze-in-stages..."
>>>
>>> I vote for the second option.
>>
>> Makes sense. 
> 
> It seems like there is some support for adding "When used in conjunction
> with --analyze in stages..." to the beginning of the sentence.  I'll give
> it another day or two for any further discussion before committing.

Here is what I have staged for commit.

-- 
nathan

  34. Re: vacuumdb changes for stats import/export

    Corey Huinker <corey.huinker@gmail.com> — 2025-07-30T17:31:07Z

    >
> > It seems like there is some support for adding "When used in conjunction
> > with --analyze in stages..." to the beginning of the sentence.  I'll give
> > it another day or two for any further discussion before committing.
>
> Here is what I have staged for commit.
>

+1

  35. Re: vacuumdb changes for stats import/export

    Jeff Davis <pgsql@j-davis.com> — 2025-07-30T17:44:00Z

    On Wed, 2025-07-30 at 12:21 -0500, Nathan Bossart wrote:
> Here is what I have staged for commit.

That's more clear to me. I also like that it shows that the options
work well together, because that was not obvious before.

Regards,
	Jeff Davis

  36. Re: vacuumdb changes for stats import/export

    Nathan Bossart <nathandbossart@gmail.com> — 2025-07-30T18:05:48Z

    Committed.

-- 
nathan