Thanks for looking!
> I don't think this is needed anymore, as of
> 45762084545ec14dbbe66ace1d69d7e89f8978ac.
Correct. That showed up after my last rebase.
> > +/*
> > + * Callback to generate a normalized version of the query string that will be used to
> > + * represent all similar queries.
> > + *
>
> I don't think the term "Callback" makes sense here - I think you could
> just keep the original wording.
This was a remnant of my earlier experimentation. I removed.
A few notes on the comments:
- * Note that the normalized representation may well vary depending on
- * just which "equivalent" query is used to create the hashtable entry.
- * We assume this is OK.
This was in the original generate_normalize_query and since it mentions
hashtable entry, I moved the comment (in-spirit) to where pg_stat_statements
calls GenerateNormailzeQuery
* If query_loc > 0, then "query" has been advanced by that much compared to
* the original string start, as is the case with multi-statement strings, so
* we need to translate the provided locations to compensate. (This lets us
* avoid re-scanning statements before the one of interest, so it's worth doing.)
*
This comment was originally duplicated in both SetConstantLengths, so I
just kept it as-is in SetConstantLengths and added a shorter reference in
GenerateNormalizeQuery
Also, this comment "It is the caller's job to ensure that the string
is a valid SQL statement..."
made more sense in GenerateNormalizeQuery rather than SetConstantLengths, since
GenerateNormalizeQuery is the public function.
> In 0002:
> You could use palloc_array for locs here.
done.
> I think we should update the comment here to reflect the fact that
> we're no longer modifying JumbleState.
done.
> Otherwise these patches look good - it'd be nice to still get this
> into 19 so we have less code duplication across the different
> extensions working with normalized query text.
I agree!
v6 addresses the comments.
--
Sami Imseih
Amazon Web Services (AWS)
