Re: ANALYZE ONLY

From: Michael Harris <harmic(at)gmail(dot)com>
To: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, postgres(at)jeltef(dot)nl, ilya(dot)evdokimov(at)tantorlabs(dot)com
Subject: Re: ANALYZE ONLY
Date: 2024-09-01 01:31:48
Message-ID: CADofcAWmRMdP81OuP+Gm-BKmSPKu1Bxx6w-pY7GE8qT_f_mX=w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello Atsushi & Melih

Thank you both for your further feedback.

On Thu, 29 Aug 2024 at 21:31, Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com> wrote:
> I believe moving "[ ONLY ]" to the definitions of table_and_columns below, as you did with "[ * ]", might be better to be consistent with other places (see [1])

Agreed. I have updated this.

>> + if ((options & VACOPT_VACUUM) && is_partitioned_table && ! include_children)
>
> There are also some issues with coding conventions in some places (e.g. the space between "!" and "include_children" abode). I think running pgindent would resolve such issue in most places.

I fixed that extra space, and then ran pgindent. It did not report any
more problems.

On Fri, 30 Aug 2024 at 16:45, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com> wrote:
> -- https://www.postgresql.org/docs/devel/progress-reporting.html
> > Note that when ANALYZE is run on a partitioned table, all of its
> > partitions are also recursively analyzed.
>
> Should we also note this is the default, i.e. not using ONLY option
> behavior here?

> -- https://www.postgresql.org/docs/devel/ddl-partitioning.html
> > If you are using manual VACUUM or ANALYZE commands, don't forget that
> > you need to run them on each child table individually. A command like:
> >
> > ANALYZE measurement;
> > will only process the root table.
>
> This part also should be modified, shouldn't it?

Agreed. I have updated both of these.

> When running ANALYZE VERBOSE ONLY on a partition table, the INFO message
> is like this:
>
> =# ANALYZE VERBOSE ONLY only_parted;
> INFO: analyzing "public.only_parted" inheritance tree
>
> I may be wrong, but 'inheritance tree' makes me feel it includes child
> tables.
> Removing 'inheritance tree' and just describing the table name as below
> might be better:
>
> INFO: analyzing "public.only_parted"

I'm not sure about that one. If I understand the source correctly,
that particular progress
message tells the user that the system is gathering stats from the inheritance
tree in order to update the stats of the given table, not that it is
actually updating
the stats of the descendant tables.

When analyzing an inheritance structure with the ONLY you see
something like this:

=> ANALYZE VERBOSE ONLY only_inh_parent;
INFO: analyzing "public.only_inh_parent"
INFO: "only_inh_parent": scanned 0 of 0 pages, containing 0 live rows
and 0 dead rows; 0 rows in sample, 0 estimated total rows
INFO: analyzing "public.only_inh_parent" inheritance tree
INFO: "only_inh_child": scanned 1 of 1 pages, containing 3 live rows
and 0 dead rows; 3 rows in sample, 3 estimated total rows
ANALYZE

The reason you don't see the first one for partitioned tables is that
it corresponds
to sampling the contents of the parent table itself, which in the case
of a partitioned
table is guaranteed to be empty, so it is skipped.

I agree the text could be confusing, and in fact is probably confusing
even today
without the ONLY keyword, but I'm not sure what would be the best
wording to cover both the partitioned and inherited cases.

v3 of the patch is attached, and I will submit it to the commitfest.

Attachment Content-Type Size
v3-0001-Implementation-of-the-ONLY-feature.patch application/octet-stream 15.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Harris 2024-09-01 01:41:45 Re: ANALYZE ONLY
Previous Message Tomas Vondra 2024-08-31 20:37:31 Re: index prefetching