Thanks for the review. On Wed, 21 May 2025 at 03:35, Sami Imseih <samims...@gmail.com> wrote: > 2/ Should "DatumGetInt64(hash_any_extended(......" be turned into a > macro since it's used in > several places? Also, we can add a comment in the macro such as > " > Output the queryId as an int64 rather than a uint64, to match the > BIGINT column used to emit > queryId in system views > " > > I think this comment is needed to address the confusion that is the > original subject of this thread. Otherwise, > the confusion will be moved from pg_stat_statements.c to queryjumblefuncs.c
I ended up adding a comment in the Query struct detailing why queryId is signed. I imagine, as time passes, we might forget why we did that as hashed values are more naturally unsigned. I think it's wrong to add comments in DoJumble() to mention details about what the calling function does. I think the fact that DoJumble() now returns int64 should be a good enough explanation as to why we want to get the hash value in signed form. David
