On Sun, Mar 6, 2016 at 11:02 PM, Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote: > At Sat, 5 Mar 2016 16:41:29 +0900, Amit Langote <amitlangot...@gmail.com> > wrote in <CA+HiwqHTeuqWMc+ktneGqFdJMRXD=syncgu0914tvxaahof...@mail.gmail.com> >> On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote <amitlangot...@gmail.com> wrote: >> > So, I took the Vinayak's latest patch and rewrote it a little >> ... >> > I broke it into two: >> > >> > 0001-Provide-a-way-for-utility-commands-to-report-progres.patch >> > 0002-Implement-progress-reporting-for-VACUUM-command.patch >> >> Oops, unamended commit messages in those patches are misleading. So, >> please find attached corrected versions. > > The 0001-P.. adds the following interface functions. > > +extern void pgstat_progress_set_command(BackendCommandType cmdtype); > +extern void pgstat_progress_set_command_target(Oid objid); > +extern void pgstat_progress_update_param(int index, uint32 val); > +extern void pgstat_reset_local_progress(void); > +extern int pgstat_progress_get_num_param(BackendCommandType cmdtype); > > I don't like to treat the target object id differently from other > parameters. It could not be needed at all, or could be needed two > or more in contrast. Although oids are not guaranteed to fit > uint32, we have already stored BlockNumber there.
Well... There's not much point in deciding that the parameters are uint32, because we don't have that type at the SQL level. pgstat_progress_update_param() really ought to take either int32 or int64 as an argument, because that's what we can actually handle from SQL, and it seems pretty clear that int64 is better since otherwise we can't fit, among other things, a block number. Given that, I tend to think that treating the command target specially and passing that as an OID is reasonable. We're not going to be able to pass variable-sized arrays through this mechanism, ever, because our shared memory segment doesn't work like that. And it seems to me that nearly every command somebody might want to report progress on will touch, basically, one relation a a time. So I don't see the harm in hardcoding that idea into the facility. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
