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

On Sat, Mar 18, 2017 at 12:46 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> 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?
>

Thanks for reviewing - updated patch attached, comments below.

> 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.
>

Fixed.

> 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.
>

I've changed this, although I've kept this somewhat dynamic, to avoid
having an accidental overrun in edge cases:

1 + floor(log10(jstate->clocations_count +
jstate->highest_extern_param_id));

> 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.
>

Updated the docs to clarify this.

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

I already had a separate patch that adds more regression tests (0000),
including one for prepared statements in the initial email.

Merged together now into one patch to keep it simple, and added another
constant value to the prepared statement test case. I've also included
a commit message in the patch file, if helpful.

Thanks,
Lukas

--
Lukas Fittl