> > One minor comment is to change is to remove the "let's save it" but
> > in the comments, as we are no longer saving a last_loc.
>
> Ah, yeah, that's a good change. I have pushed it now.
Thanks for pushing!
I will take 0002 on in a separate thread as there seems to
be agreement on doing somet
On 2025-Oct-28, Sami Imseih wrote:
> getting rid of last_loc makes sense because the list is sorted. I like
> this, definitely cleaner.
>
> One minor comment is to change is to remove the "let's save it" but
> in the comments, as we are no longer saving a last_loc.
Ah, yeah, that's a good change
On Tue, Oct 28, 2025 at 12:31:23PM +0200, Alvaro Herrera wrote:
> Hmm, yeah, now that you mention this, it seems rather odd that
> pgss_store() is messing with (modifying) the jstate it is given. If we
> had multiple modules using a planner_hook to interact with the query
> representation, what ea
>
> > > On Fri, Oct 24, 2025 at 07:04:59PM -0500, Sami Imseih wrote:
> > > v4 corrects some code comments.
> >
> > The fix in the first patch looks good, thanks.
>
> Yeah, I think this general idea is sensible. However, I think we should
> take it one step further and just remove last_loc entirely
On 2025-Oct-26, Dmitry Dolgov wrote:
> > On Fri, Oct 24, 2025 at 07:04:59PM -0500, Sami Imseih wrote:
> > v4 corrects some code comments.
>
> The fix in the first patch looks good, thanks.
Yeah, I think this general idea is sensible. However, I think we should
take it one step further and just
On 2025-Oct-28, Michael Paquier wrote:
> FWIW, the line defining the separation between pg_stat_statements.c
> and queryjumblefuncs.c should rely on a pretty simple concept:
> JumbleState can be written only by queryjumblefuncs.c and
> src/backend/nodes/, and should remain in an untouched constant
On Mon, Oct 27, 2025 at 02:34:50PM -0500, Sami Imseih wrote:
> The excellent question raised earlier is whether the fix should be
> applied in pg_stat_statements.c or queryjumblefuncs.c. To me, this
> suggests that pg_stat_statements, as an extension, is dealing with code
> it should not be respons
Thanks for reviewing!
>Not so sure about the
> refactoring in the second patch -- generate_normalized_query is being
> used only in one place, in pg_stat_statements, moving it out somewhere
> else without having other clients seems to be questionable to me.
The excellent question raised earlier i
> On Fri, Oct 24, 2025 at 07:04:59PM -0500, Sami Imseih wrote:
> v4 corrects some code comments.
The fix in the first patch looks good, thanks. Not so sure about the
refactoring in the second patch -- generate_normalized_query is being
used only in one place, in pg_stat_statements, moving it out s
v4 corrects some code comments.
Also, I created a CF entry [0].
[0] https://commitfest.postgresql.org/patch/6167/
v4-0001-pg_stat_statements-Fix-duplicate-constant-locatio.patch
Description: Binary data
v4-0002-pg_stat_statements-move-out-jumble-specific-routi.patch
Description: Binary data
> > > > Having said
> > > > that it seems that another solution would be to check for duplicated
> > > > constants
> > > > in fill_in_constant_length
> > >
> > > Yes, I started thinking along these lines as well, where we check for
> > > duplicates
> > > in fill_in_constant_length; or after jumbli
> > > Having said
> > > that it seems that another solution would be to check for duplicated
> > > constants
> > > in fill_in_constant_length
> >
> > Yes, I started thinking along these lines as well, where we check for
> > duplicates
> > in fill_in_constant_length; or after jumbling, we de-duplic
> On Fri, Oct 24, 2025 at 09:17:22AM -0500, Sami Imseih wrote:
> > Having said
> > that it seems that another solution would be to check for duplicated
> > constants
> > in fill_in_constant_length
>
> Yes, I started thinking along these lines as well, where we check for
> duplicates
> in fill_in_
> This sort of short cut the verification for duplicated constants. Having said
> that it seems that another solution would be to check for duplicated constants
> in fill_in_constant_lengths just before skipping squashed constants and reset
> the length if needed.
Hi,
yes, I was not convinced my
> On Thu, Oct 23, 2025 at 07:49:55PM -0500, Sami Imseih wrote:
> I believe the correct answer is to fix this in queryjumblefuncs.c
> and more specifically inside _jumbleElements. We should not
> record the same location twice. We can do this by tracking the
> start location of the last recorded co
> In PG18 gthere is bug either in pg_stat_statements.c, either in
> queryjumblefuncs.c.
>
> Repro (you need Postgres build with asserts and pg_stat_statements
> included in shared_preload_librarties:
>
> create function f(a integer[], out x integer, out y integer) returns
> record as $$
16 matches
Mail list logo
