public inbox for [email protected]
help / color / mirror / Atom feedRe: pg_stat_statements and "IN" conditions
14+ messages / 6 participants
[nested] [flat]
* Re: pg_stat_statements and "IN" conditions
@ 2022-03-14 15:02 Robert Haas <[email protected]>
2022-03-14 15:10 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: Robert Haas @ 2022-03-14 15:02 UTC (permalink / raw)
To: Dmitry Dolgov <[email protected]>; +Cc: Tom Lane <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
On Mon, Mar 14, 2022 at 10:57 AM Dmitry Dolgov <[email protected]> wrote:
> Well, yeah, the commit message is somewhat clumsy in this regard. It
> works almost in the way you've described, except if the list is all
> constants and long enough to satisfy the threshold then *first N
> elements (where N == threshold) will be jumbled -- to leave at least
> some traces of it in pgss.
But that seems to me to be a thing we would not want. Why do you think
otherwise?
> I'm not sure if I follow the last point. WHERE x in (1,3) and x =
> any(array[1,3]) are two different things for sure, but in which way are
> they going to be mixed together because of this change? My goal was to
> make only the following transformation, without leaving any uncertainty:
>
> WHERE x in (1, 2, 3, 4, 5) -> WHERE x in (1, 2, ...)
> WHERE x = any(array[1, 2, 3, 4, 5]) -> WHERE x = any(array[1, 2, ...])
I understand. I think it might be OK to transform both of those
things, but I don't think it's very clear either from the comments or
the nonexistent documentation that both of those cases are affected --
and I think that needs to be clear. Not sure exactly how to do that,
just saying that we can't add behavior unless it will be clear to
users what the behavior is.
> Sure, I'll add documentation. To be honest I'm not targeting PG15 with
> this, just want to make some progress.
wfm!
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
@ 2022-03-14 15:10 ` Dmitry Dolgov <[email protected]>
1 sibling, 0 replies; 14+ messages in thread
From: Dmitry Dolgov @ 2022-03-14 15:10 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tom Lane <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
> On Mon, Mar 14, 2022 at 11:02:16AM -0400, Robert Haas wrote:
> On Mon, Mar 14, 2022 at 10:57 AM Dmitry Dolgov <[email protected]> wrote:
> > Well, yeah, the commit message is somewhat clumsy in this regard. It
> > works almost in the way you've described, except if the list is all
> > constants and long enough to satisfy the threshold then *first N
> > elements (where N == threshold) will be jumbled -- to leave at least
> > some traces of it in pgss.
>
> But that seems to me to be a thing we would not want. Why do you think
> otherwise?
Hm. Well, if the whole list would be not jumbled, the transformation
would look like this, right?
WHERE x in (1, 2, 3, 4, 5) -> WHERE x in (...)
Leaving some number of original elements in place gives some clue for
the reader about at least what type of data the array has contained.
Which hopefully makes it a bit easier to identify even in the collapsed
form:
WHERE x in (1, 2, 3, 4, 5) -> WHERE x in (1, 2, ...)
> > I'm not sure if I follow the last point. WHERE x in (1,3) and x =
> > any(array[1,3]) are two different things for sure, but in which way are
> > they going to be mixed together because of this change? My goal was to
> > make only the following transformation, without leaving any uncertainty:
> >
> > WHERE x in (1, 2, 3, 4, 5) -> WHERE x in (1, 2, ...)
> > WHERE x = any(array[1, 2, 3, 4, 5]) -> WHERE x = any(array[1, 2, ...])
>
> I understand. I think it might be OK to transform both of those
> things, but I don't think it's very clear either from the comments or
> the nonexistent documentation that both of those cases are affected --
> and I think that needs to be clear. Not sure exactly how to do that,
> just saying that we can't add behavior unless it will be clear to
> users what the behavior is.
Yep, got it.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
@ 2022-03-14 15:23 ` Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Tom Lane @ 2022-03-14 15:23 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
Robert Haas <[email protected]> writes:
> On Mon, Mar 14, 2022 at 10:57 AM Dmitry Dolgov <[email protected]> wrote:
>> I'm not sure if I follow the last point. WHERE x in (1,3) and x =
>> any(array[1,3]) are two different things for sure, but in which way are
>> they going to be mixed together because of this change? My goal was to
>> make only the following transformation, without leaving any uncertainty:
>>
>> WHERE x in (1, 2, 3, 4, 5) -> WHERE x in (1, 2, ...)
>> WHERE x = any(array[1, 2, 3, 4, 5]) -> WHERE x = any(array[1, 2, ...])
> I understand. I think it might be OK to transform both of those
> things, but I don't think it's very clear either from the comments or
> the nonexistent documentation that both of those cases are affected --
> and I think that needs to be clear.
We've transformed IN(...) to ANY(ARRAY[...]) at the parser stage for a
long time, and this has been visible to users of either EXPLAIN or
pg_stat_statements for the same length of time. I doubt people are
going to find that surprising. Even if they do, it's not the query
jumbler's fault.
I do find it odd that the proposed patch doesn't cause the *entire*
list to be skipped over. That seems like extra complexity and confusion
to no benefit.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
@ 2022-03-14 15:33 ` Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Dolgov @ 2022-03-14 15:33 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
> On Mon, Mar 14, 2022 at 11:23:17AM -0400, Tom Lane wrote:
> Robert Haas <[email protected]> writes:
>
> I do find it odd that the proposed patch doesn't cause the *entire*
> list to be skipped over. That seems like extra complexity and confusion
> to no benefit.
That's a bit surprising for me, I haven't even thought that folks could
think this is an odd behaviour. As I've mentioned above, the original
idea was to give some clues about what was inside the collapsed array,
but if everyone finds it unnecessary I can of course change it.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
@ 2022-03-14 15:38 ` Tom Lane <[email protected]>
2022-03-14 15:51 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lane @ 2022-03-14 15:38 UTC (permalink / raw)
To: Dmitry Dolgov <[email protected]>; +Cc: Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
Dmitry Dolgov <[email protected]> writes:
> On Mon, Mar 14, 2022 at 11:23:17AM -0400, Tom Lane wrote:
>> I do find it odd that the proposed patch doesn't cause the *entire*
>> list to be skipped over. That seems like extra complexity and confusion
>> to no benefit.
> That's a bit surprising for me, I haven't even thought that folks could
> think this is an odd behaviour. As I've mentioned above, the original
> idea was to give some clues about what was inside the collapsed array,
> but if everyone finds it unnecessary I can of course change it.
But if what we're doing is skipping over an all-Consts list, then the
individual Consts would be elided from the pg_stat_statements entry
anyway, no? All that would remain is information about how many such
Consts there were, which is exactly the information you want to drop.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
@ 2022-03-14 15:51 ` Dmitry Dolgov <[email protected]>
2022-03-26 17:40 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Dolgov @ 2022-03-14 15:51 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
> On Mon, Mar 14, 2022 at 11:38:23AM -0400, Tom Lane wrote:
> Dmitry Dolgov <[email protected]> writes:
> > On Mon, Mar 14, 2022 at 11:23:17AM -0400, Tom Lane wrote:
> >> I do find it odd that the proposed patch doesn't cause the *entire*
> >> list to be skipped over. That seems like extra complexity and confusion
> >> to no benefit.
>
> > That's a bit surprising for me, I haven't even thought that folks could
> > think this is an odd behaviour. As I've mentioned above, the original
> > idea was to give some clues about what was inside the collapsed array,
> > but if everyone finds it unnecessary I can of course change it.
>
> But if what we're doing is skipping over an all-Consts list, then the
> individual Consts would be elided from the pg_stat_statements entry
> anyway, no? All that would remain is information about how many such
> Consts there were, which is exactly the information you want to drop.
Hm, yes, you're right. I guess I was thinking about this more like about
shortening some text with ellipsis, but indeed no actual Consts will end
up in the result anyway. Thanks for clarification, will modify the
patch!
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:51 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
@ 2022-03-26 17:40 ` Dmitry Dolgov <[email protected]>
2022-07-24 10:06 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Dolgov @ 2022-03-26 17:40 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
> On Mon, Mar 14, 2022 at 04:51:50PM +0100, Dmitry Dolgov wrote:
> > On Mon, Mar 14, 2022 at 11:38:23AM -0400, Tom Lane wrote:
> > Dmitry Dolgov <[email protected]> writes:
> > > On Mon, Mar 14, 2022 at 11:23:17AM -0400, Tom Lane wrote:
> > >> I do find it odd that the proposed patch doesn't cause the *entire*
> > >> list to be skipped over. That seems like extra complexity and confusion
> > >> to no benefit.
> >
> > > That's a bit surprising for me, I haven't even thought that folks could
> > > think this is an odd behaviour. As I've mentioned above, the original
> > > idea was to give some clues about what was inside the collapsed array,
> > > but if everyone finds it unnecessary I can of course change it.
> >
> > But if what we're doing is skipping over an all-Consts list, then the
> > individual Consts would be elided from the pg_stat_statements entry
> > anyway, no? All that would remain is information about how many such
> > Consts there were, which is exactly the information you want to drop.
>
> Hm, yes, you're right. I guess I was thinking about this more like about
> shortening some text with ellipsis, but indeed no actual Consts will end
> up in the result anyway. Thanks for clarification, will modify the
> patch!
Here is another iteration. Now the patch doesn't leave any trailing
Consts in the normalized query, and contains more documentation. I hope
it's getting better.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:51 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-26 17:40 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
@ 2022-07-24 10:06 ` Dmitry Dolgov <[email protected]>
2022-09-16 18:25 ` Re:pg_stat_statements and "IN" conditions Sergei Kornilov <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Dolgov @ 2022-07-24 10:06 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
> On Sat, Mar 26, 2022 at 06:40:35PM +0100, Dmitry Dolgov wrote:
> > On Mon, Mar 14, 2022 at 04:51:50PM +0100, Dmitry Dolgov wrote:
> > > On Mon, Mar 14, 2022 at 11:38:23AM -0400, Tom Lane wrote:
> > > Dmitry Dolgov <[email protected]> writes:
> > > > On Mon, Mar 14, 2022 at 11:23:17AM -0400, Tom Lane wrote:
> > > >> I do find it odd that the proposed patch doesn't cause the *entire*
> > > >> list to be skipped over. That seems like extra complexity and confusion
> > > >> to no benefit.
> > >
> > > > That's a bit surprising for me, I haven't even thought that folks could
> > > > think this is an odd behaviour. As I've mentioned above, the original
> > > > idea was to give some clues about what was inside the collapsed array,
> > > > but if everyone finds it unnecessary I can of course change it.
> > >
> > > But if what we're doing is skipping over an all-Consts list, then the
> > > individual Consts would be elided from the pg_stat_statements entry
> > > anyway, no? All that would remain is information about how many such
> > > Consts there were, which is exactly the information you want to drop.
> >
> > Hm, yes, you're right. I guess I was thinking about this more like about
> > shortening some text with ellipsis, but indeed no actual Consts will end
> > up in the result anyway. Thanks for clarification, will modify the
> > patch!
>
> Here is another iteration. Now the patch doesn't leave any trailing
> Consts in the normalized query, and contains more documentation. I hope
> it's getting better.
Hi,
Here is the rebased version, with no other changes.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re:pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:51 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-26 17:40 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-07-24 10:06 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
@ 2022-09-16 18:25 ` Sergei Kornilov <[email protected]>
2022-09-24 14:07 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Sergei Kornilov @ 2022-09-16 18:25 UTC (permalink / raw)
To: Dmitry Dolgov <[email protected]>; +Cc: Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
Hello!
Unfortunately the patch needs another rebase due to the recent split of guc.c (0a20ff54f5e66158930d5328f89f087d4e9ab400)
I'm reviewing a patch on top of a previous commit and noticed a failed test:
# Failed test 'no parameters missing from postgresql.conf.sample'
# at t/003_check_guc.pl line 82.
# got: '1'
# expected: '0'
# Looks like you failed 1 test of 3.
t/003_check_guc.pl ..............
The new option has not been added to the postgresql.conf.sample
PS: I would also like to have such a feature. It's hard to increase pg_stat_statements.max or lose some entries just because some ORM sends requests with a different number of parameters.
regards, Sergei
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:51 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-26 17:40 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-07-24 10:06 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-09-16 18:25 ` Re:pg_stat_statements and "IN" conditions Sergei Kornilov <[email protected]>
@ 2022-09-24 14:07 ` Dmitry Dolgov <[email protected]>
2022-09-24 23:59 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Dolgov @ 2022-09-24 14:07 UTC (permalink / raw)
To: Sergei Kornilov <[email protected]>; +Cc: Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
> On Fri, Sep 16, 2022 at 09:25:13PM +0300, Sergei Kornilov wrote:
> Hello!
>
> Unfortunately the patch needs another rebase due to the recent split of guc.c (0a20ff54f5e66158930d5328f89f087d4e9ab400)
>
> I'm reviewing a patch on top of a previous commit and noticed a failed test:
>
> # Failed test 'no parameters missing from postgresql.conf.sample'
> # at t/003_check_guc.pl line 82.
> # got: '1'
> # expected: '0'
> # Looks like you failed 1 test of 3.
> t/003_check_guc.pl ..............
>
> The new option has not been added to the postgresql.conf.sample
>
> PS: I would also like to have such a feature. It's hard to increase pg_stat_statements.max or lose some entries just because some ORM sends requests with a different number of parameters.
Thanks! I'll post the rebased version soon.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:51 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-26 17:40 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-07-24 10:06 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-09-16 18:25 ` Re:pg_stat_statements and "IN" conditions Sergei Kornilov <[email protected]>
2022-09-24 14:07 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
@ 2022-09-24 23:59 ` Dmitry Dolgov <[email protected]>
2023-01-27 14:45 ` Re: pg_stat_statements and "IN" conditions vignesh C <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Dolgov @ 2022-09-24 23:59 UTC (permalink / raw)
To: Sergei Kornilov <[email protected]>; +Cc: Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
> On Sat, Sep 24, 2022 at 04:07:14PM +0200, Dmitry Dolgov wrote:
> > On Fri, Sep 16, 2022 at 09:25:13PM +0300, Sergei Kornilov wrote:
> > Hello!
> >
> > Unfortunately the patch needs another rebase due to the recent split of guc.c (0a20ff54f5e66158930d5328f89f087d4e9ab400)
> >
> > I'm reviewing a patch on top of a previous commit and noticed a failed test:
> >
> > # Failed test 'no parameters missing from postgresql.conf.sample'
> > # at t/003_check_guc.pl line 82.
> > # got: '1'
> > # expected: '0'
> > # Looks like you failed 1 test of 3.
> > t/003_check_guc.pl ..............
> >
> > The new option has not been added to the postgresql.conf.sample
> >
> > PS: I would also like to have such a feature. It's hard to increase pg_stat_statements.max or lose some entries just because some ORM sends requests with a different number of parameters.
>
> Thanks! I'll post the rebased version soon.
And here it is.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:51 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-26 17:40 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-07-24 10:06 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-09-16 18:25 ` Re:pg_stat_statements and "IN" conditions Sergei Kornilov <[email protected]>
2022-09-24 14:07 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-09-24 23:59 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
@ 2023-01-27 14:45 ` vignesh C <[email protected]>
2023-01-29 12:22 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: vignesh C @ 2023-01-27 14:45 UTC (permalink / raw)
To: Dmitry Dolgov <[email protected]>; +Cc: Sergei Kornilov <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
On Sun, 25 Sept 2022 at 05:29, Dmitry Dolgov <[email protected]> wrote:
>
> > On Sat, Sep 24, 2022 at 04:07:14PM +0200, Dmitry Dolgov wrote:
> > > On Fri, Sep 16, 2022 at 09:25:13PM +0300, Sergei Kornilov wrote:
> > > Hello!
> > >
> > > Unfortunately the patch needs another rebase due to the recent split of guc.c (0a20ff54f5e66158930d5328f89f087d4e9ab400)
> > >
> > > I'm reviewing a patch on top of a previous commit and noticed a failed test:
> > >
> > > # Failed test 'no parameters missing from postgresql.conf.sample'
> > > # at t/003_check_guc.pl line 82.
> > > # got: '1'
> > > # expected: '0'
> > > # Looks like you failed 1 test of 3.
> > > t/003_check_guc.pl ..............
> > >
> > > The new option has not been added to the postgresql.conf.sample
> > >
> > > PS: I would also like to have such a feature. It's hard to increase pg_stat_statements.max or lose some entries just because some ORM sends requests with a different number of parameters.
> >
> > Thanks! I'll post the rebased version soon.
The patch does not apply on top of HEAD as in [1], please post a rebased patch:
=== Applying patches on top of PostgreSQL commit ID
456fa635a909ee36f73ca84d340521bd730f265f ===
=== applying patch
./v9-0001-Prevent-jumbling-of-every-element-in-ArrayExpr.patch
....
can't find file to patch at input line 746
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/src/backend/utils/misc/queryjumble.c
b/src/backend/utils/misc/queryjumble.c
|index a67487e5fe..063b4be725 100644
|--- a/src/backend/utils/misc/queryjumble.c
|+++ b/src/backend/utils/misc/queryjumble.c
--------------------------
No file to patch. Skipping patch.
8 out of 8 hunks ignored
can't find file to patch at input line 913
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/src/include/utils/queryjumble.h b/src/include/utils/queryjumble.h
|index 3c2d9beab2..b50cc42d4e 100644
|--- a/src/include/utils/queryjumble.h
|+++ b/src/include/utils/queryjumble.h
--------------------------
No file to patch. Skipping patch.
[1] - http://cfbot.cputube.org/patch_41_2837.log
Regards,
Vignesh
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: pg_stat_statements and "IN" conditions
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:23 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2022-03-14 15:51 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-03-26 17:40 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-07-24 10:06 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-09-16 18:25 ` Re:pg_stat_statements and "IN" conditions Sergei Kornilov <[email protected]>
2022-09-24 14:07 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2022-09-24 23:59 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2023-01-27 14:45 ` Re: pg_stat_statements and "IN" conditions vignesh C <[email protected]>
@ 2023-01-29 12:22 ` Dmitry Dolgov <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Dolgov @ 2023-01-29 12:22 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Sergei Kornilov <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
> On Fri, Jan 27, 2023 at 08:15:29PM +0530, vignesh C wrote:
> The patch does not apply on top of HEAD as in [1], please post a rebased patch:
Thanks. I think this one should do the trick.
Attachments:
[text/x-diff] v10-0001-Prevent-jumbling-of-every-element-in-ArrayExpr.patch (34.0K, ../../[email protected]/2-v10-0001-Prevent-jumbling-of-every-element-in-ArrayExpr.patch)
download | inline diff:
From 3c51561ddaecdbc82842fae4fab74cc33526f17c Mon Sep 17 00:00:00 2001
From: Dmitrii Dolgov <[email protected]>
Date: Sun, 24 Jul 2022 11:43:25 +0200
Subject: [PATCH v10] Prevent jumbling of every element in ArrayExpr
pg_stat_statements produces multiple entries for queries like
SELECT something FROM table WHERE col IN (1, 2, 3, ...)
depending on number of parameters, because every element of ArrayExpr is
jumbled. In certain situations it's undesirable, especially if the list
becomes too large.
Make Const expressions contribute nothing to the jumble hash if they're
a part of an ArrayExpr, which length is larger than specified threshold.
Allow to configure the threshold via the new GUC const_merge_threshold
with the default value zero, which disables this feature.
Reviewed-by: Zhihong Yu, Sergey Dudoladov, Robert Haas, Tom Lane
Tested-by: Chengxi Sun
---
.../expected/pg_stat_statements.out | 412 ++++++++++++++++++
.../pg_stat_statements/pg_stat_statements.c | 33 +-
.../sql/pg_stat_statements.sql | 107 +++++
doc/src/sgml/config.sgml | 26 ++
doc/src/sgml/pgstatstatements.sgml | 28 +-
src/backend/nodes/queryjumblefuncs.c | 105 ++++-
src/backend/utils/misc/guc_tables.c | 13 +
src/backend/utils/misc/postgresql.conf.sample | 2 +-
src/include/nodes/queryjumble.h | 5 +-
9 files changed, 712 insertions(+), 19 deletions(-)
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index 9ac5c87c3a..f18f34ae5b 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -1141,4 +1141,416 @@ SELECT COUNT(*) FROM pg_stat_statements WHERE query LIKE '%SELECT GROUPING%';
2
(1 row)
+--
+-- Consts merging
+--
+CREATE TABLE test_merge (id int, data int);
+-- IN queries
+-- No merging
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+--------------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6) | 1
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7) | 1
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8) | 1
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8, $9) | 1
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(7 rows)
+
+-- Normal
+SET const_merge_threshold = 5;
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3) | 1
+ SELECT * FROM test_merge WHERE id IN (...) | 5
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(4 rows)
+
+-- On the merge threshold
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1
+ SELECT * FROM test_merge WHERE id IN (...) | 6
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(4 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 6
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 5
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+-- With gaps on the threshold
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1
+ SELECT * FROM test_merge WHERE id IN (...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(4 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 2
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 2
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+-- test constants after merge
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) and data = 2;
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) and data = $3 | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+-- On table, numeric type causes every constant being wrapped into functions.
+CREATE TABLE test_merge_numeric (id int, data numeric(5, 2));
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge_numeric WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ id | data
+----+------
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge_numeric WHERE id IN (...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+-- Test find_const_walker
+WITH cte AS (
+ SELECT 'const' as const FROM test_merge
+)
+SELECT ARRAY['a', 'b', 'c', const::varchar] AS result
+FROM cte;
+ result
+--------
+(0 rows)
+
+RESET const_merge_threshold;
DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index ad1fe44496..b26ae1f234 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2666,6 +2666,9 @@ generate_normalized_query(JumbleState *jstate, const char *query,
n_quer_loc = 0, /* Normalized query byte location */
last_off = 0, /* Offset from start for previous tok */
last_tok_len = 0; /* Length (in bytes) of that tok */
+ bool skip = false; /* Signals that certain constants are
+ merged together and have to be skipped */
+
/*
* Get constants' lengths (core system only gives us locations). Note
@@ -2689,7 +2692,6 @@ generate_normalized_query(JumbleState *jstate, const char *query,
{
int off, /* Offset from start for cur tok */
tok_len; /* Length (in bytes) of that tok */
-
off = jstate->clocations[i].location;
/* Adjust recorded location if we're dealing with partial string */
off -= query_loc;
@@ -2704,12 +2706,31 @@ generate_normalized_query(JumbleState *jstate, const char *query,
len_to_wrt -= last_tok_len;
Assert(len_to_wrt >= 0);
- memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt);
- n_quer_loc += len_to_wrt;
- /* And insert a param symbol in place of the constant token */
- n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d",
- i + 1 + jstate->highest_extern_param_id);
+ /* Normal path, non merged constant */
+ if (!jstate->clocations[i].merged)
+ {
+ memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt);
+ n_quer_loc += len_to_wrt;
+
+ /* And insert a param symbol in place of the constant token */
+ n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d",
+ i + 1 + jstate->highest_extern_param_id);
+
+ /* In case previous constants were merged away, stop doing that */
+ if (skip)
+ skip = false;
+ }
+ /* The firsts merged constant */
+ else if (!skip)
+ {
+ memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt);
+ n_quer_loc += len_to_wrt;
+
+ /* Skip the following until a non merged constant appear */
+ skip = true;
+ n_quer_loc += sprintf(norm_query + n_quer_loc, "...");
+ }
quer_loc = off + tok_len;
last_off = off;
diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
index 8f5c866225..8f9d284ed3 100644
--- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql
+++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
@@ -464,4 +464,111 @@ SELECT (
SELECT COUNT(*) FROM pg_stat_statements WHERE query LIKE '%SELECT GROUPING%';
+--
+-- Consts merging
+--
+CREATE TABLE test_merge (id int, data int);
+
+-- IN queries
+
+-- No merging
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- Normal
+SET const_merge_threshold = 5;
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- On the merge threshold
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- With gaps on the threshold
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- test constants after merge
+SELECT pg_stat_statements_reset();
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) and data = 2;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- On table, numeric type causes every constant being wrapped into functions.
+CREATE TABLE test_merge_numeric (id int, data numeric(5, 2));
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge_numeric WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- Test find_const_walker
+WITH cte AS (
+ SELECT 'const' as const FROM test_merge
+)
+SELECT ARRAY['a', 'b', 'c', const::varchar] AS result
+FROM cte;
+
+RESET const_merge_threshold;
+
DROP EXTENSION pg_stat_statements;
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f985afc009..270107926c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8278,6 +8278,32 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</listitem>
</varlistentry>
+ <varlistentry id="guc-const-merge-threshold" xreflabel="const_merge_treshold">
+ <term><varname>const_merge_threshold</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>const_merge_threshold</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies the minimal length of an array to be eligible for constants
+ collapsing. Normally every element of an array contributes to a query
+ identifier, which means the same query containing an array of constants
+ could get multiple different identifiers, depending of size of the
+ array. If this parameter is nonzero, the array contains only constants
+ and it's length is larger than <varname> const_merge_threshold </varname>,
+ then array elements will contribure nothing to the query identifier.
+ Thus the query will get the same identifier no matter how many constants
+ it contains.
+
+ Zero turns off collapsing, and it is the default value.
+
+ The <xref linkend="pgstatstatements"/> extension will represent such
+ collapsed constants via <literal>'(...)'</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml
index efc36da602..f7e2e9fe85 100644
--- a/doc/src/sgml/pgstatstatements.sgml
+++ b/doc/src/sgml/pgstatstatements.sgml
@@ -519,10 +519,30 @@
<para>
In some cases, queries with visibly different texts might get merged into a
single <structname>pg_stat_statements</structname> entry. Normally this will happen
- only for semantically equivalent queries, but there is a small chance of
- hash collisions causing unrelated queries to be merged into one entry.
- (This cannot happen for queries belonging to different users or databases,
- however.)
+ only for semantically equivalent queries, for example when queries are
+ different only in values of constants they use. Another valid possibility for
+ merging queries into a single <structname>pg_stat_statements</structname>
+ entry is when <xref linkend="guc-const-merge-threshold"/> is nonzero and the
+ queries contain an array with more than <varname>const_merge_threshold</varname>
+ constants in it:
+
+<screen>
+=# SET const_merge_threshold = 5;
+=# SELECT pg_stat_statements_reset();
+=# SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+=# SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7);
+=# SELECT query, calls FROM pg_stat_statements;
+-[ RECORD 1 ]------------------------------
+query | SELECT * FROM test WHERE a IN (...)
+calls | 2
+-[ RECORD 2 ]------------------------------
+query | SELECT pg_stat_statements_reset()
+calls | 1
+</screen>
+
+ But there is a small chance of hash collisions causing unrelated queries to
+ be merged into one entry. (This cannot happen for queries belonging to
+ different users or databases, however.)
</para>
<para>
diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c
index 16084842a3..1ea1cc66f8 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -42,6 +42,9 @@
/* GUC parameters */
int compute_query_id = COMPUTE_QUERY_ID_AUTO;
+/* Minimal numer of constants in an array after which they will be merged */
+int const_merge_threshold = 0;
+
/* True when compute_query_id is ON, or AUTO and a module requests them */
bool query_id_enabled = false;
@@ -53,7 +56,8 @@ static void JumbleQueryInternal(JumbleState *jstate, Query *query);
static void JumbleRangeTable(JumbleState *jstate, List *rtable);
static void JumbleRowMarks(JumbleState *jstate, List *rowMarks);
static void JumbleExpr(JumbleState *jstate, Node *node);
-static void RecordConstLocation(JumbleState *jstate, int location);
+static void JumbleExprList(JumbleState *jstate, List *node);
+static void RecordConstLocation(JumbleState *jstate, int location, bool merged);
/*
* Given a possibly multi-statement source string, confine our attention to the
@@ -120,7 +124,7 @@ JumbleQuery(Query *query, const char *querytext)
jstate->jumble_len = 0;
jstate->clocations_buf_size = 32;
jstate->clocations = (LocationLen *)
- palloc(jstate->clocations_buf_size * sizeof(LocationLen));
+ palloc0(jstate->clocations_buf_size * sizeof(LocationLen));
jstate->clocations_count = 0;
jstate->highest_extern_param_id = 0;
@@ -343,6 +347,90 @@ JumbleRowMarks(JumbleState *jstate, List *rowMarks)
}
}
+/*
+ * Jubmle a list of expressions
+ *
+ * This function enforces const_merge_threshold limitation, i.e. if the
+ * provided list contains only constant expressions and its length is greater
+ * than or equal to const_merge_threshold, such list will not contribute to
+ * jumble. Otherwise it falls back to JumbleExpr.
+ */
+static void
+JumbleExprList(JumbleState *jstate, List *elements)
+{
+ ListCell *temp;
+ Node *firstExpr = NULL;
+ bool allConst = true;
+
+ if (elements == NULL)
+ return;
+
+ if (const_merge_threshold == 0)
+ {
+ /* Merging is disabled, process everything one by one. */
+ JumbleExpr(jstate, (Node *) elements);
+ return;
+ }
+
+ if (elements->length < const_merge_threshold)
+ {
+ /* The list is not large enough to collapse it. */
+ JumbleExpr(jstate, (Node *) elements);
+ return;
+ }
+
+ /* Guard against stack overflow due to overly complex expressions */
+ check_stack_depth();
+
+ firstExpr = linitial(elements);
+
+ /*
+ * We always emit the node's NodeTag, then any additional fields that are
+ * considered significant, and then we recurse to any child nodes.
+ */
+ APP_JUMB(elements->type);
+
+ /*
+ * If the first expression is a constant, verify if the following elements
+ * are constants as well. If yes, the list is eligible for collapsing --
+ * mark it as merged and return from the function.
+ */
+ if (IsA(firstExpr, Const))
+ {
+ foreach(temp, elements)
+ {
+ Node *expr = (Node *) lfirst(temp);
+
+ if (!IsA(expr, Const))
+ {
+ allConst = false;
+ break;
+ }
+ }
+
+ if (allConst)
+ {
+ Const *firstConst = (Const *) firstExpr;
+ Const *lastConst = llast_node(Const, elements);
+
+ /*
+ * First and last constants are needed to identify which part of
+ * the query to skip in generate_normalized_query.
+ */
+ RecordConstLocation(jstate, firstConst->location, true);
+ RecordConstLocation(jstate, lastConst->location, true);
+ return;
+ }
+ }
+
+ /*
+ * If we end up here, it means no constants merging is possible, process
+ * the list as usual.
+ */
+ JumbleExpr(jstate, (Node *) elements);
+ return;
+}
+
/*
* Jumble an expression tree
*
@@ -392,7 +480,7 @@ JumbleExpr(JumbleState *jstate, Node *node)
/* We jumble only the constant's type, not its value */
APP_JUMB(c->consttype);
/* Also, record its parse location for query normalization */
- RecordConstLocation(jstate, c->location);
+ RecordConstLocation(jstate, c->location, false);
}
break;
case T_Param:
@@ -581,7 +669,7 @@ JumbleExpr(JumbleState *jstate, Node *node)
}
break;
case T_ArrayExpr:
- JumbleExpr(jstate, (Node *) ((ArrayExpr *) node)->elements);
+ JumbleExprList(jstate, (List *) ((ArrayExpr *) node)->elements);
break;
case T_RowExpr:
JumbleExpr(jstate, (Node *) ((RowExpr *) node)->args);
@@ -835,11 +923,13 @@ JumbleExpr(JumbleState *jstate, Node *node)
}
/*
- * Record location of constant within query string of query tree
- * that is currently being walked.
+ * Record location of constant within query string of query tree that is
+ * currently being walked. Merged argument signals that the constant do not
+ * contribute to the jumble hash, and any reader of constants array may want to
+ * use this information to represent such constants differently.
*/
static void
-RecordConstLocation(JumbleState *jstate, int location)
+RecordConstLocation(JumbleState *jstate, int location, bool merged)
{
/* -1 indicates unknown or undefined location */
if (location >= 0)
@@ -854,6 +944,7 @@ RecordConstLocation(JumbleState *jstate, int location)
sizeof(LocationLen));
}
jstate->clocations[jstate->clocations_count].location = location;
+ jstate->clocations[jstate->clocations_count].merged = merged;
/* initialize lengths to -1 to simplify third-party module usage */
jstate->clocations[jstate->clocations_count].length = -1;
jstate->clocations_count++;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 4ac808ed22..663aded290 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3467,6 +3467,19 @@ struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"const_merge_threshold", PGC_SUSET, STATS_MONITORING,
+ gettext_noop("Sets the minimal numer of constants in an array"
+ " after which they will be merged"),
+ gettext_noop("Computing query id for an array of constants"
+ " will produce the same id for all arrays with length"
+ " larger than this value. Zero turns off merging."),
+ },
+ &const_merge_threshold,
+ 0, 0, INT_MAX,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index d06074b86f..0594eb17b2 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -627,7 +627,7 @@
#log_parser_stats = off
#log_planner_stats = off
#log_executor_stats = off
-
+#const_merge_threshold = 0
#------------------------------------------------------------------------------
# AUTOVACUUM
diff --git a/src/include/nodes/queryjumble.h b/src/include/nodes/queryjumble.h
index 204b8f74fd..4410e2cf61 100644
--- a/src/include/nodes/queryjumble.h
+++ b/src/include/nodes/queryjumble.h
@@ -15,6 +15,7 @@
#define QUERYJUBLE_H
#include "nodes/parsenodes.h"
+#include "nodes/nodeFuncs.h"
/*
* Struct for tracking locations/lengths of constants during normalization
@@ -23,6 +24,8 @@ typedef struct LocationLen
{
int location; /* start offset in query text */
int length; /* length in bytes, or -1 to ignore */
+ bool merged; /* whether or not the location was marked as
+ not contributing to jumble */
} LocationLen;
/*
@@ -61,7 +64,7 @@ enum ComputeQueryIdType
/* GUC parameters */
extern PGDLLIMPORT int compute_query_id;
-
+extern PGDLLIMPORT int const_merge_threshold;
extern const char *CleanQuerytext(const char *query, int *location, int *len);
extern JumbleState *JumbleQuery(Query *query, const char *querytext);
--
2.32.0
^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH 1/3] fix CREATE INDEX progress report with nested partitions
@ 2023-01-31 15:13 Ilya Gladyshev <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Ilya Gladyshev @ 2023-01-31 15:13 UTC (permalink / raw)
The progress reporting was added in v12 (ab0dfc961) but the original
patch didn't seem to consider the possibility of nested partitioning.
When called recursively, DefineIndex() would clobber the number of
completed partitions, and it was possible to end up with the TOTAL
counter greater than the DONE counter.
This clarifies/re-defines that the progress reporting counts both direct
and indirect children, but doesn't count intermediate partitioned tables:
- The TOTAL counter is set once at the start of the command.
- For indexes which are newly-built, the recursively-called
DefineIndex() increments the DONE counter.
- For pre-existing indexes which are ATTACHed rather than built,
DefineIndex() increments the DONE counter, and if the attached index is
partitioned, the counter is incremented to account for each of its leaf
partitions.
Author: Ilya Gladyshev
Reviewed-By: Justin Pryzby, Tomas Vondra, Dean Rasheed, Alvaro Herrera, Matthias van de Meent
Discussion: https://www.postgresql.org/message-id/flat/a15f904a70924ffa4ca25c3c744cff31e0e6e143.camel%40gmail.co...
---
doc/src/sgml/monitoring.sgml | 10 ++-
src/backend/commands/indexcmds.c | 70 +++++++++++++++++--
src/backend/utils/activity/backend_progress.c | 28 ++++++++
src/include/utils/backend_progress.h | 1 +
4 files changed, 102 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 1756f1a4b67..fa139dcece7 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6601,7 +6601,10 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para>
<para>
When creating an index on a partitioned table, this column is set to
- the total number of partitions on which the index is to be created.
+ the total number of partitions on which the index is to be created or attached.
+ In the case of intermediate partitioned tables, this includes both
+ direct and indirect partitions, but excludes the intermediate
+ partitioned tables themselves.
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
</para></entry>
</row>
@@ -6612,7 +6615,10 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para>
<para>
When creating an index on a partitioned table, this column is set to
- the number of partitions on which the index has been created.
+ the number of partitions on which the index has been created or attached.
+ In the case of intermediate partitioned tables, this includes both
+ direct and indirect partitions, but excludes the intermediate
+ partitioned tables themselves.
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
</para></entry>
</row>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 16ec0b114e6..84c84c41acc 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -130,6 +130,30 @@ typedef struct ReindexErrorInfo
char relkind;
} ReindexErrorInfo;
+
+/*
+ * Count the number of direct and indirect leaf partitions, excluding foreign
+ * tables.
+ */
+static int
+count_leaf_partitions(Oid relid)
+{
+ int nleaves = 0;
+ List *childs = find_all_inheritors(relid, NoLock, NULL);
+ ListCell *lc;
+
+ foreach(lc, childs)
+ {
+ Oid partrelid = lfirst_oid(lc);
+
+ if (RELKIND_HAS_STORAGE(get_rel_relkind(partrelid)))
+ nleaves++;
+ }
+
+ list_free(childs);
+ return nleaves;
+}
+
/*
* CheckIndexCompatible
* Determine whether an existing index definition is compatible with a
@@ -1219,8 +1243,18 @@ DefineIndex(Oid relationId,
Relation parentIndex;
TupleDesc parentDesc;
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
- nparts);
+ /*
+ * Set the total number of partitions at the start of the command,
+ * but don't change it when being called recursively.
+ */
+ if (!OidIsValid(parentIndexId))
+ {
+ int total_parts;
+
+ total_parts = count_leaf_partitions(relationId);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
+ total_parts);
+ }
/* Make a local copy of partdesc->oids[], just for safety */
memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
@@ -1250,6 +1284,7 @@ DefineIndex(Oid relationId,
{
Oid childRelid = part_oids[i];
Relation childrel;
+ char child_relkind;
Oid child_save_userid;
int child_save_sec_context;
int child_save_nestlevel;
@@ -1259,6 +1294,7 @@ DefineIndex(Oid relationId,
bool found = false;
childrel = table_open(childRelid, lockmode);
+ child_relkind = RelationGetForm(childrel)->relkind;
GetUserIdAndSecContext(&child_save_userid,
&child_save_sec_context);
@@ -1431,9 +1467,27 @@ DefineIndex(Oid relationId,
SetUserIdAndSecContext(child_save_userid,
child_save_sec_context);
}
+ else
+ {
+ int attached_parts;
+
+ /*
+ * Avoid the overhead of counting partitions when that
+ * can't apply.
+ */
+ attached_parts = RELKIND_HAS_PARTITIONS(child_relkind) ?
+ count_leaf_partitions(childRelid) : 1;
+
+ /*
+ * If a pre-existing index was attached, the progress
+ * report is updated here. If the index was partitioned,
+ * all the children that were counted towards
+ * PROGRESS_CREATEIDX_PARTITIONS_TOTAL are counted as
+ * done.
+ */
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, attached_parts);
+ }
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- i + 1);
free_attrmap(attmap);
}
@@ -1484,9 +1538,15 @@ DefineIndex(Oid relationId,
/* Close the heap and we're done, in the non-concurrent case */
table_close(rel, NoLock);
- /* If this is the top-level index, we're done. */
+ /*
+ * If this is the top-level index, we're done. When called recursively
+ * for child tables, the done partition counter is incremented now,
+ * rather than in the caller.
+ */
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
+ else
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
return address;
}
diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c
index d96af812b19..2a9994b98fd 100644
--- a/src/backend/utils/activity/backend_progress.c
+++ b/src/backend/utils/activity/backend_progress.c
@@ -58,6 +58,34 @@ pgstat_progress_update_param(int index, int64 val)
PGSTAT_END_WRITE_ACTIVITY(beentry);
}
+/*-----------
+ * pgstat_progress_incr_param() -
+ *
+ * Increment index'th member in st_progress_param[] of the current backend.
+ *-----------
+ */
+void
+pgstat_progress_incr_param(int index, int64 incr)
+{
+ volatile PgBackendStatus *beentry = MyBEEntry;
+ int64 val;
+
+ Assert(index >= 0 && index < PGSTAT_NUM_PROGRESS_PARAM);
+
+ if (!beentry || !pgstat_track_activities)
+ return;
+
+ /*
+ * Because no other process should write to this backend's own status, we
+ * can read its value from shared memory without needing to loop to ensure
+ * its consistency.
+ */
+ val = beentry->st_progress_param[index];
+ val += incr;
+
+ pgstat_progress_update_param(index, val);
+}
+
/*-----------
* pgstat_progress_update_multi_param() -
*
diff --git a/src/include/utils/backend_progress.h b/src/include/utils/backend_progress.h
index 005e5d75ab6..a84752ade99 100644
--- a/src/include/utils/backend_progress.h
+++ b/src/include/utils/backend_progress.h
@@ -36,6 +36,7 @@ typedef enum ProgressCommandType
extern void pgstat_progress_start_command(ProgressCommandType cmdtype,
Oid relid);
extern void pgstat_progress_update_param(int index, int64 val);
+extern void pgstat_progress_incr_param(int index, int64 incr);
extern void pgstat_progress_update_multi_param(int nparam, const int *index,
const int64 *val);
extern void pgstat_progress_end_command(void);
--
2.25.1
--GBDnBH7+ZvLx8QD4
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0002-assertions-for-progress-reporting.patch"
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2023-01-31 15:13 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-03-14 15:02 Re: pg_stat_statements and "IN" conditions Robert Haas <[email protected]>
2022-03-14 15:10 ` Dmitry Dolgov <[email protected]>
2022-03-14 15:23 ` Tom Lane <[email protected]>
2022-03-14 15:33 ` Dmitry Dolgov <[email protected]>
2022-03-14 15:38 ` Tom Lane <[email protected]>
2022-03-14 15:51 ` Dmitry Dolgov <[email protected]>
2022-03-26 17:40 ` Dmitry Dolgov <[email protected]>
2022-07-24 10:06 ` Dmitry Dolgov <[email protected]>
2022-09-16 18:25 ` Sergei Kornilov <[email protected]>
2022-09-24 14:07 ` Dmitry Dolgov <[email protected]>
2022-09-24 23:59 ` Dmitry Dolgov <[email protected]>
2023-01-27 14:45 ` vignesh C <[email protected]>
2023-01-29 12:22 ` Dmitry Dolgov <[email protected]>
2023-01-31 15:13 [PATCH 1/3] fix CREATE INDEX progress report with nested partitions Ilya Gladyshev <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox