On Wed, Jan 9, 2019 at 10:06 AM Michael Paquier <mich...@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
