From: Shinoda, Noriyoshi (SXD Japan FSIP) <noriyoshi.shinoda@hpe.com>
To: Jeff Davis <pgsql@j-davis.com>
To: jian he <jian.universality@gmail.com>
To: Corey Huinker <corey.huinker@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: Bruce Momjian <bruce@momjian.us>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Nathan Bossart <nathandbossart@gmail.com>
Cc: Magnus Hagander <magnus@hagander.net>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: alvherre@alvh.no-ip.org <alvherre@alvh.no-ip.org>
Subject: RE: Statistics Import and Export
Date: Sat, 12 Oct 2024 13:02:55 +0000
Message-ID: <DM4PR84MB17345E2DFF28A5557B7CBC3CEE7A2@DM4PR84MB1734.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <e0660898b696b0bb77030f8b850d662ad3c663d2.camel@j-davis.com>
References: <CACJufxHeRNu9EJM-XsfhuSo87_3oRaPMuPcqdsH_D9-W4dLJOg@mail.gmail.com>
<e0660898b696b0bb77030f8b850d662ad3c663d2.camel@j-davis.com>
Hi,
Thank you for developing this great feature. I have tested the committed feature.
The manual for the pg_set_relation_stats function says the following:
"The value of relpages must be greater than or equal to 0"
However, this function seems to accept -1 for the relpages parameter. Below is an example of execution:
---
postgres=> CREATE TABLE data1(c1 INT PRIMARY KEY, c2 VARCHAR(10));
CREATE TABLE
postgres=> SELECT pg_set_relation_stats('data1', relpages=>-1);
pg_set_relation_stats
-----------------------
t
(1 row)
postgres=> SELECT relname, relpages FROM pg_class WHERE relname='data1';
relname | relpages
---------+----------
data1 | -1
(1 row)
---
The attached patch modifies the pg_set_relation_stats function to work as described in the manual.
Regards,
Noriyoshi Shinoda
-----Original Message-----
From: Jeff Davis <pgsql@j-davis.com>
Sent: Saturday, October 12, 2024 8:11 AM
To: jian he <jian.universality@gmail.com>; Corey Huinker <corey.huinker@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; Bruce Momjian <bruce@momjian.us>; Tom Lane <tgl@sss.pgh.pa.us>; Nathan Bossart <nathandbossart@gmail.com>; Magnus Hagander <magnus@hagander.net>; Stephen Frost <sfrost@snowman.net>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; Peter Smith <smithpb2250@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Tomas Vondra <tomas.vondra@enterprisedb.com>; alvherre@alvh.no-ip.org
Subject: Re: Statistics Import and Export
On Mon, 2024-09-23 at 08:57 +0800, jian he wrote:
> newtup = heap_modify_tuple_by_cols(ctup, tupdesc, ncols, replaces,
> nulls);
>
> you just directly declared "bool nulls[3] = {false, false, false};"
Those must be false (not NULL), because in pg_class those are non-NULL attributes. They must be set to something whenever we update.
> if any of (RELPAGES_ARG, RELTUPLES_ARG, RELALLVISIBLE_ARG) is null,
> should you set that null[position] to true?
If the corresponding SQL argument is NULL, we leave the existing value unchanged, we don't set it to NULL.
> otherwise, i am confused with the variable nulls.
>
> Looking at other usage of heap_modify_tuple_by_cols, "ncols" cannot be
> dynamic, it should be a fixed value?
> The current implementation works, because the (bool[3] nulls) is
> always false, never changed.
> if nulls becomes {false, false, true} then "ncols" must be 3, cannot
> be 2.
heap_modify_tuple_by_cols() uses ncols to specify the length of the values/isnull arrays. The "replaces" is an array of attribute numbers to replace (in contrast to plain heap_modify_tuple(), which uses an array of booleans).
We are going to replace a maximum of 3 attributes, so the arrays have a maximum size of 3. Predeclaring the arrays to be 3 elements is just fine even if we only use the first 1-2 elements -- it avoids a needless heap allocation/free.
Regards,
Jeff Davis
Attachments:relpages_stat_update_v1.diff (application/octet-stream, 633B) downloaddiff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.cindex ffa3d83a87..26f15061e8 100644--- a/src/backend/statistics/relation_stats.c+++ b/src/backend/statistics/relation_stats.c@@ -99,11 +99,11 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
{
int32 relpages = PG_GETARG_INT32(RELPAGES_ARG);
- if (relpages < -1)+ if (relpages < 0)
{
ereport(elevel,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("relpages cannot be < -1")));+ errmsg("relpages cannot be < 0")));
table_close(crel, RowExclusiveLock);
return false;
}
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noriyoshi.shinoda@hpe.com, pgsql@j-davis.com, jian.universality@gmail.com, corey.huinker@gmail.com, boekewurm+postgres@gmail.com, bruce@momjian.us, tgl@sss.pgh.pa.us, nathandbossart@gmail.com, magnus@hagander.net, sfrost@snowman.net, ashutosh.bapat.oss@gmail.com, smithpb2250@gmail.com, pgsql-hackers@lists.postgresql.org, tomas.vondra@enterprisedb.com, alvherre@alvh.no-ip.org
Subject: RE: Statistics Import and Export
In-Reply-To: <DM4PR84MB17345E2DFF28A5557B7CBC3CEE7A2@DM4PR84MB1734.NAMPRD84.PROD.OUTLOOK.COM>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox