Re: [PATCH] Use $ parameters as replacement characters for pg_stat_statements

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Mar 13, 2017 at 6:05 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I wonder if it would improve matters to use $n, but starting with the
>> first number after the actual external Param symbols in the query.

> That sounds pretty sensible, although I guess it's got the weakness
> that you might get confused about which kind of $n you are looking at.
> However, I'd be inclined to deem that a fairly minor weakness and go
> with it: just because somebody could hypothetically get confused
> doesn't mean that real people will.

So it turns out this discussion just reinvented the alternative that
Lukas had in his 0002 proposal. Are there any remaining objections
to proceeding with that approach?

In a quick read of the patch, I note that it falsifies and fails to
update the header comment for generate_normalized_query:

* *query_len_p contains the input string length, and is updated with
* the result string length (which cannot be longer) on exit.

It doesn't look like the calling code depends (any more?) on the
assumption that the string doesn't get longer, but it would be good
to double-check that.

This bit seems much more expensive and complicated than necessary:

+ /* Accomodate the additional query replacement characters */
+ norm_query_buflen = query_len;
+ for (i = 0; i < jstate->clocations_count; i++)
+ {
+ norm_query_buflen += floor(log10(abs(i + 1 + jstate->highest_extern_param_id))) + 1;
+ }

I'd just add, say, "10 * clocations_count" to the allocation length.
We can afford to waste a few bytes on this transient copy of the query
text.

The documentation is overly vague about what the parameter symbols are,
in particular failing to explain that their numbers start from after
the last $n occurring in the original query text.

It seems like a good idea to have a regression test case demonstrating
that, too.

regards, tom lane