Re: [HACKERS] [PATCH] Use $ parameters as replacement characters for pg_stat_statements - Mailing list pgsql-hackers

From Lukas Fittl
Subject Re: [HACKERS] [PATCH] Use $ parameters as replacement characters for pg_stat_statements
Date
Msg-id CAP53PkzHsQQZQ7G0RbYKAA+h0QLvsVyC=SYbzEgxBesiHu876A@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] [PATCH] Use $ parameters as replacement characters for pg_stat_statements  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [PATCH] Use $ parameters as replacement characters for pg_stat_statements  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Sat, Mar 18, 2017 at 12:46 PM, Tom Lane <tgl@sss.pgh.pa.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
Attachment

pgsql-hackers by date:

Previous
From: Craig Ringer
Date:
Subject: Re: [HACKERS] Logical decoding on standby
Next
From: Amit Langote
Date:
Subject: Re: [HACKERS] UPDATE of partition key