On Wed, Jan 9, 2019 at 10:06 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Jan 08, 2019 at 06:46:11PM +0000, Bossart, Nathan wrote:
> > This was done in order to maintain the current behavior that
> > appendQualifiedRelation() gives us. I found that skipping the
> > search_path handling here forced us to specify the schema in the
> > argument for --table in most cases. At the very least, I could add a
> > comment here to highlight the importance of fully qualifying
> > everything in the catalog query. What do you think?
>
> A comment sounds like a good thing. And we really shouldn't hijack
> search_path even for one query...
>
> > Looks good to me, except for one small thing in the documentation:
> >
> > + <para>
> > + Disable all page-skipping behavior during processing based on
> > + the visibility map, similarly to the option
> > + <literal>DISABLE_PAGE_SKIPPING</literal> for <command>VACUUM</command>.
> > + </para>
> >
> > I think the "similarly to the option" part is slightly misleading.
> > It's not just similar, it _is_ using that option in the generated
> > commands. Perhaps we could point to the VACUUM documentation for more
> > information about this one.
>
> Sure. If you have any suggestions, please feel free. Adding a link
> to VACUUM documentation sounds good to me, as long as we avoid
> duplicating the description related to DISABLE_PAGE_SKIPPING on the
> VACUUM page.
>
> > Good point. I think allowing multiple different relation size options
> > here would be confusing, too (e.g. --min-relation-size versus
> > --min-total-relation-size). IMO pg_total_relation_size() is the way
> > to go here, as we'll most likely need to process the indexes and TOAST
> > tables, too. If/when there is a DISABLE_INDEX_CLEANUP option for
> > VACUUM, we'd then want to use pg_table_size() when --min-relation-size
> > and --disable-index-cleanup are used together in vacuumdb.
> > Thoughts?
>
> Yes, using pg_total_relation_size() looks like the best option to me
> here as well, still this does not make me 100% comfortable from the
> user perspective.
Agreed.
Since pg_(total)_realtion_size() returns 0 for parent table the
specifying the parent table to vacuumdb with --min-relation-size
always does nothing. Maybe we will need to deal with this case when a
function returning whole partitoned table size is introduced.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
