Hi,
On 2022-12-10 20:32:06 +1300, David Rowley wrote:
> @@ -20,13 +20,14 @@
> typedef struct CommandTagBehavior
> {
> const char *name;
> + const uint8 namelen;
Perhaps worth adding a comment noting that namelen is the length without the
null byte?
> +static inline Size
> +make_completion_tag(char *buff, const QueryCompletion *qc,
> + bool force_undecorated_output)
> +{
> + CommandTag tag = qc->commandTag;
> + Size taglen;
> + const char *tagname = GetCommandTagNameAndLen(tag, &taglen);
> + char *bufp;
> +
> + /*
> + * We assume the tagname is plain ASCII and therefore requires no
> encoding
> + * conversion.
> + */
> + memcpy(buff, tagname, taglen + 1);
> + bufp = buff + taglen;
> +
> + /* ensure that the tagname isn't long enough to overrun the buffer */
> + Assert(taglen <= COMPLETION_TAG_BUFSIZE - MAXINT8LEN - 4);
> +
> + /*
> + * In PostgreSQL versions 11 and earlier, it was possible to create a
> + * table WITH OIDS. When inserting into such a table, INSERT used to
> + * include the Oid of the inserted record in the completion tag. To
> + * maintain compatibility in the wire protocol, we now write a "0" (for
> + * InvalidOid) in the location where we once wrote the new record's Oid.
> + */
> + if (command_tag_display_rowcount(tag) && !force_undecorated_output)
This does another external function call to cmdtag.c...
What about moving make_completion_tag() to cmdtag.c? Then we could just get
the entire CommandTagBehaviour struct at once. It's not super pretty to pass
QueryCompletion to a routine in cmdtag.c, but it's not awful. And if we deem
it problematic, we could just pass qc->commandTag, qc->nprocessed as a
separate arguments.
I wonder if any of the other GetCommandTagName() would benefit noticably from
not having to compute the length. I guess the calls
set_ps_display(GetCommandTagName()) calls in exec_simple_query() and
exec_execute_message() might, although set_ps_display() isn't exactly zero
overhead. But I do see it show up as a few percent in profiles, with the
biggest contributor being the call to strlen.
Greetings,
Andres Freund
