public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 9/9] Fix crash in multirange_intersect_multirange_internal
5+ messages / 4 participants
[nested] [flat]
* [PATCH 9/9] Fix crash in multirange_intersect_multirange_internal
@ 2019-09-26 21:01 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Alvaro Herrera @ 2019-09-26 21:01 UTC (permalink / raw)
---
src/backend/utils/adt/multirangetypes.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/multirangetypes.c b/src/backend/utils/adt/multirangetypes.c
index c85f5dc90d..7717acb3b1 100644
--- a/src/backend/utils/adt/multirangetypes.c
+++ b/src/backend/utils/adt/multirangetypes.c
@@ -1123,7 +1123,8 @@ multirange_intersect_multirange(PG_FUNCTION_ARGS)
MultirangeType *
multirange_intersect_multirange_internal(Oid mltrngtypoid, TypeCacheEntry *rangetyp,
- int32 range_count1, RangeType **ranges1, int32 range_count2, RangeType **ranges2)
+ int32 range_count1, RangeType **ranges1,
+ int32 range_count2, RangeType **ranges2)
{
RangeType *r1;
RangeType *r2;
@@ -1132,6 +1133,9 @@ multirange_intersect_multirange_internal(Oid mltrngtypoid, TypeCacheEntry *range
int32 i1;
int32 i2;
+ if (range_count1 == 0 || range_count2 == 0)
+ return make_multirange(mltrngtypoid, rangetyp, 0, NULL);
+
/*
* Worst case is a stitching pattern like this:
*
--
2.17.1
--G4iJoqBmSsgzjUCe--
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry
@ 2023-03-28 03:36 Kyotaro Horiguchi <[email protected]>
2023-03-28 05:23 ` Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Michael Paquier <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Kyotaro Horiguchi @ 2023-03-28 03:36 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]
At Tue, 28 Mar 2023 07:38:25 +0900, Michael Paquier <[email protected]> wrote in
> On Mon, Mar 27, 2023 at 10:24:46AM -0400, Melanie Plageman wrote:
> > I do like/prefer "block read requests" and
> > "blocks requested found in cache"
> > Though, now I fear my initial complaint may have been a bit pedantic.
>
> That's fine. Let's ask for extra opinions, then.
>
> So, have others an opinion to share here?
I do not have a strong preference for the wording, but consistency is
important. IMHO simply swapping out a few words won't really improve
things.
I found that commit ddfc2d9a37 removed the descriptions for
pg_stat_get_blocks_fetched and pg_stat_get_blocks_hit. Right before
that commit, monitoring.sgml had these lines:
- <function>pg_stat_get_blocks_fetched</function> minus
- <function>pg_stat_get_blocks_hit</function> gives the number of kernel
- <function>read()</> calls issued for the table, index, or
- database; the number of actual physical reads is usually
- lower due to kernel-level buffering. The <literal>*_blks_read</>
- statistics columns use this subtraction, i.e., fetched minus hit.
The commit then added the following sentence to the description for
pg_statio_all_tables.heap_blks_read.
> <entry>Number of disk blocks read from this table.
> This value can also be returned by directly calling
> the <function>pg_stat_get_blocks_fetched</function> and
> <function>pg_stat_get_blocks_hit</function> functions and
> subtracting the results.</entry>
Later, in 5f2b089387 it twas revised as:
+ <entry>Number of disk blocks read in this database</entry>
This revision lost the explantion regarding the relationship among
fetch, hit and read, as it became hidden in the views' definitions.
As the result, in the current state, it doesn't make sense to just add
a description for pg_stat_get_xact_blocks_fetched().
The confusion stems from the inconsistency between the views and
underlying functions related to block reads and hits. If we add
descriptions for the two functions, we should also explain their
relationship. Otherwise, it might be better to add the functions
pg_stat_*_blocks_read() instead.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry
2023-03-28 03:36 Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Kyotaro Horiguchi <[email protected]>
@ 2023-03-28 05:23 ` Michael Paquier <[email protected]>
2023-03-28 05:49 ` Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Drouvot, Bertrand <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Michael Paquier @ 2023-03-28 05:23 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]
On Tue, Mar 28, 2023 at 12:36:15PM +0900, Kyotaro Horiguchi wrote:
> I found that commit ddfc2d9a37 removed the descriptions for
> pg_stat_get_blocks_fetched and pg_stat_get_blocks_hit. Right before
> that commit, monitoring.sgml had these lines:
>
> - <function>pg_stat_get_blocks_fetched</function> minus
> - <function>pg_stat_get_blocks_hit</function> gives the number of kernel
> - <function>read()</> calls issued for the table, index, or
> - database; the number of actual physical reads is usually
> - lower due to kernel-level buffering. The <literal>*_blks_read</>
> - statistics columns use this subtraction, i.e., fetched minus hit.
>
> The commit then added the following sentence to the description for
> pg_statio_all_tables.heap_blks_read.
>
> Later, in 5f2b089387 it twas revised as:
> + <entry>Number of disk blocks read in this database</entry>
Yeah, maybe adding something like that at the bottom of the table for
stat functions, telling that the difference is the number of read()
calls, may help. Perhaps also adding a mention that these are used in
none of the existing system views..
> The confusion stems from the inconsistency between the views and
> underlying functions related to block reads and hits. If we add
> descriptions for the two functions, we should also explain their
> relationship. Otherwise, it might be better to add the functions
> pg_stat_*_blocks_read() instead.
I am not sure that we really need to get down to that as this holds
the same meaning as the current system views showing read as the
difference between fetched and hit.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../ZCJ53%[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry
2023-03-28 03:36 Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Kyotaro Horiguchi <[email protected]>
2023-03-28 05:23 ` Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Michael Paquier <[email protected]>
@ 2023-03-28 05:49 ` Drouvot, Bertrand <[email protected]>
2023-03-28 06:16 ` Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Michael Paquier <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Drouvot, Bertrand @ 2023-03-28 05:49 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]
Hi,
On 3/28/23 7:23 AM, Michael Paquier wrote:
> On Tue, Mar 28, 2023 at 12:36:15PM +0900, Kyotaro Horiguchi wrote:
>> I found that commit ddfc2d9a37 removed the descriptions for
>> pg_stat_get_blocks_fetched and pg_stat_get_blocks_hit. Right before
>> that commit, monitoring.sgml had these lines:
>>
>> - <function>pg_stat_get_blocks_fetched</function> minus
>> - <function>pg_stat_get_blocks_hit</function> gives the number of kernel
>> - <function>read()</> calls issued for the table, index, or
>> - database; the number of actual physical reads is usually
>> - lower due to kernel-level buffering. The <literal>*_blks_read</>
>> - statistics columns use this subtraction, i.e., fetched minus hit.
>>
>> The commit then added the following sentence to the description for
>> pg_statio_all_tables.heap_blks_read.
>>
>> Later, in 5f2b089387 it twas revised as:
>> + <entry>Number of disk blocks read in this database</entry>
>
> Yeah, maybe adding something like that at the bottom of the table for
> stat functions, telling that the difference is the number of read()
> calls, may help. Perhaps also adding a mention that these are used in
> none of the existing system views..
>
>> The confusion stems from the inconsistency between the views and
>> underlying functions related to block reads and hits. If we add
>> descriptions for the two functions, we should also explain their
>> relationship.
I agree that adding more explanation would help and avoid confusion.
What about something like?
for pg_stat_get_xact_blocks_fetched(): "block read requests for table or index, in the current
transaction. This number minus pg_stat_get_xact_blocks_hit() gives the number of kernel
read() calls."
pg_stat_get_xact_blocks_hit(): "block read requests for table or index, in the current
transaction, found in cache (not triggering kernel read() calls)".
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry
2023-03-28 03:36 Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Kyotaro Horiguchi <[email protected]>
2023-03-28 05:23 ` Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Michael Paquier <[email protected]>
2023-03-28 05:49 ` Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Drouvot, Bertrand <[email protected]>
@ 2023-03-28 06:16 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Michael Paquier @ 2023-03-28 06:16 UTC (permalink / raw)
To: Drouvot, Bertrand <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]; [email protected]; [email protected]
On Tue, Mar 28, 2023 at 07:49:45AM +0200, Drouvot, Bertrand wrote:
> What about something like?
>
> for pg_stat_get_xact_blocks_fetched(): "block read requests for table or index, in the current
> transaction. This number minus pg_stat_get_xact_blocks_hit() gives the number of kernel
> read() calls."
>
> pg_stat_get_xact_blocks_hit(): "block read requests for table or index, in the current
> transaction, found in cache (not triggering kernel read() calls)".
Something among these lines within the table would be also OK by me.
Horiguchi-san or Melanie-san, perhaps you have counter-proposals?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../ZCKGROQ%[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2023-03-28 06:16 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 21:01 [PATCH 9/9] Fix crash in multirange_intersect_multirange_internal Alvaro Herrera <[email protected]>
2023-03-28 03:36 Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Kyotaro Horiguchi <[email protected]>
2023-03-28 05:23 ` Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Michael Paquier <[email protected]>
2023-03-28 05:49 ` Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Drouvot, Bertrand <[email protected]>
2023-03-28 06:16 ` Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry Michael Paquier <[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