Re: pg_stat_statements oddity with track = all

On Sun, Dec 27, 2020 at 9:39 AM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
>
> On Fri, Dec 04, 2020 at 06:09:13PM +0800, Julien Rouhaud wrote:
> > On Fri, Dec 04, 2020 at 12:06:10PM +0300, Sergei Kornilov wrote:
> > > Hello
> > >
> > > Seems we need also change PGSS_FILE_HEADER.
> >
> > Indeed, thanks! v2 attached.
>
> There was a conflict on PGSS_FILE_HEADER since some recent commit, v3 attached.

- *
- * Right now, this structure contains no padding. If you add any, make sure
- * to teach pgss_store() to zero the padding bytes. Otherwise, things will
- * break, because pgss_hash is created using HASH_BLOBS, and thus tag_hash
- * is used to hash this.

I don't think removing this comment completely is a good idea. Right
now it ends up there, yes, but eventually it might reach the same
state again. I think it's better to reword it based on the current
situation while keeping the part about it having to be zeroed for
padding. And maybe along with a comment at the actual memset() sites
as well?

AFAICT, it's going to store two copies of the query in the query text
file (pgss_query_texts.stat)? Can we find a way around that? Maybe by
looking up the entry with the flag set to the other value, and then
reusing that?

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/