On 2015/12/01 16:25, Kyotaro HORIGUCHI wrote:
> At Mon, 30 Nov 2015 19:10:44 -0700 (MST), Vinayak <vinpokale@gmail.com> wrote
>> Thanks for the v7.
>> Please check the comment below.
>> -Table name in the vacuum progress
>>
>> + snprintf(progress_message[0], PROGRESS_MESSAGE_LENGTH, "%s.%s",
>> schemaname,relname);
>>
>> In the vacuum progress, column table_name is showing first 30 characters of
>> table name.
>
> Yeah, it is actually restricted in that length. But if we allow
> the buffer to store whole the qualified names, it will need 64 *
> 2 + 1 +1 = 130 bytes * 10 1300 bytes for each beentry... It might
> be acceptable by others, but I don't think that is preferable..
>
> Separating namespace and relation name as below reduces the
> required length of the field but 62 bytes is still too long for
> most of the information and in turn too short for longer messages
> in some cases.
As done in the patch, the table name is stored in one of the slots of
st_progress_message which has the width limit of PROGRESS_MESSAGE_LENGTH
bytes. Whereas users of pgstat_report_progress interface could make sure
that strings of their choosing to be stored in st_progress_param slots are
within the PROGRESS_MESSAGE_LENGTH limit, the same cannot be ensured for
the table name. Maybe, the table name is a different kind of information
than other reported parameters that it could be treated specially. How
about a separate st_* member, say, st_command_target[2*NAMDATALEN+1] for
the table name? It would be reported using a separate interface, say,
pgstat_report_command_target() once the name is determined. Moreover,
subsequent pgstat_report_progress() invocations need not copy the table
name needlessly as part of copying argument values to st_progress_param
(which is a separate suggestion in its own right though).
Thanks,
Amit