On Sun, Mar 6, 2016 at 11:02 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> At Sat, 5 Mar 2016 16:41:29 +0900, Amit Langote <amitlangote09@gmail.com> wrote in
<CA+HiwqHTeuqWMc+ktneGqFdJMRXD=syncgU0914TVXaahOF56g@mail.gmail.com>
>> On Sat, Mar 5, 2016 at 4:24 PM, Amit Langote <amitlangote09@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
