Message-ID: <9a93cffe79e2be1d9450a36e773a39dc456c241e.camel@cybertec.at>
Subject: Re: analyze-in-stages post upgrade questions
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Fujii Masao <masao.fujii@gmail.com>, Mircea Cadariu
	 <cadariu.mircea@gmail.com>
Cc: "Zechman, Derek S" <Derek.S.Zechman@snapon.com>, Adrian Klaver
	 <adrian.klaver@aklaver.com>, pgsql-hackers@lists.postgresql.org
Date: Wed, 06 Aug 2025 22:52:52 +0200
In-Reply-To: <CAHGQGwEj8PCk1OVveJ_F1sj_COrV95N-mfu_tLUn4gdcAM24kA@mail.gmail.com>
References: 
	<CO1PR04MB8281387B9AD9DE30976966BBC045A@CO1PR04MB8281.namprd04.prod.outlook.com>
	 <6add2a9a-7cf2-4d1b-8f3e-2e26a7ebe883@aklaver.com>
	 <4d8122febd3007143504e4b6034b4253f7000761.camel@cybertec.at>
	 <PH0PR04MB82943D67CD179002714AA503C044A@PH0PR04MB8294.namprd04.prod.outlook.com>
	 <f2ebeef82fddaf8a8152383f8ff62e75f20aac3c.camel@cybertec.at>
	 <PH0PR04MB8294E796D6297D4E3BE4EFB6C049A@PH0PR04MB8294.namprd04.prod.outlook.com>
	 <08d943a83590308cbb9be594d80b4e19ca80e08e.camel@cybertec.at>
	 <2505eef1-b6aa-4518-baff-861a2e6a507b@gmail.com>
	 <b65126bf381d4b41f561afe691000b77ac7b8cc2.camel@cybertec.at>
	 <a018a729-9b18-444f-baed-be0d6d8ecb80@gmail.com>
	 <2c1f1834107045dfa8b32417771b56bab0cffc56.camel@cybertec.at>
	 <2cce9851-327a-4b1a-ab8e-531c2f92532b@gmail.com>
	 <CAHGQGwEvwpydO7hjnB51Q2zgS0ydv31X4HCFLNzMeqLr=poS8w@mail.gmail.com>
	 <9c9cb744-b8c7-4d75-acad-595b3faba187@gmail.com>
	 <CAHGQGwEj8PCk1OVveJ_F1sj_COrV95N-mfu_tLUn4gdcAM24kA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.56.2 (3.56.2-1.fc42) 
MIME-Version: 1.0
Archived-At: <https://www.postgresql.org/message-id/9a93cffe79e2be1d9450a36e773a39dc456c241e.camel%40cybertec.at>
Precedence: bulk

On Wed, 2025-08-06 at 23:25 +0900, Fujii Masao wrote:
> On Wed, Aug 6, 2025 at 1:01=E2=80=AFPM Mircea Cadariu <cadariu.mircea@gma=
il.com> wrote:
> > Overall, I like the change. But I have one question: should this be tre=
ated as
> > a bug fix that we back-patch to supported branches, or is it more of
> > an improvement that should only go into master?
> >=20
> > I reckon it might make sense to back-patch it to previous versions, as =
users might not upgrade always to the latest version.
>=20
> I understand your point. But on second thought, since the patch changes
> behavior, I'm leaning toward treating it as an improvement, so it should
> only go to master...

I agree that this behavior change should not be backpatched.
That is not a bugfix.

> > + /*
> > + * VACUUMing partitioned tables would be unreasonably expensive, since
> > + * that entails processing the partitions twice (once as part of the
> > + * partitioned table, once as tables in their own right) for no
> > + * benefit. But if we only ANALYZE, collecting statistics for
> > + * partitioned tables is worth the effort.
> > + */
> >=20
> > This is probably true. But isn't the main reason more about aligning wi=
th
> > the behavior of the underlying VACUUM and ANALYZE commands? As the vacu=
umdb
> > docs says, "There is no effective difference between vacuuming and anal=
yzing
> > databases via this utility and via other methods for accessing the serv=
er.",
> > so its default target objects should match: VACUUM skips partitioned ta=
bles
> > by default, while ANALYZE includes them. If that's the case, maybe the =
comment
> > should reflect that instead.
> >=20
> > I see what you mean. From that perspective, I wonder if we even need a =
comment there at all.
>=20
> Or, if we keep it, though, I'd like to update it to something like
> the following:
>=20
> --------------------
> vacuumdb should generally follow the behavior of the underlying
> VACUUM and ANALYZE commands. If analyze_only is true, process
> regular tables, materialized views, and partitioned tables, just like
> ANALYZE (with no specific target tables) does. Otherwise, process
> only regular tables and materialized views, since VACUUM skips
> partitioned tables when no target tables are specified.
> --------------------

I am fine with that suggestion.

Alternatively, my original comment could be amended with

  Besides, ANALYZE (without an option) processes partitioned tables, and
  "vacuumdb -Z" should behave like ANALYZE.

Yours,
Laurenz Albe