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 <pg...@j-davis.com> 
Sent: Saturday, October 12, 2024 8:11 AM
To: jian he <jian.universal...@gmail.com>; Corey Huinker 
<corey.huin...@gmail.com>
Cc: Matthias van de Meent <boekewurm+postg...@gmail.com>; Bruce Momjian 
<br...@momjian.us>; Tom Lane <t...@sss.pgh.pa.us>; Nathan Bossart 
<nathandboss...@gmail.com>; Magnus Hagander <mag...@hagander.net>; Stephen 
Frost <sfr...@snowman.net>; Ashutosh Bapat <ashutosh.bapat....@gmail.com>; 
Peter Smith <smithpb2...@gmail.com>; PostgreSQL Hackers 
<pgsql-hackers@lists.postgresql.org>; Tomas Vondra 
<tomas.von...@enterprisedb.com>; alvhe...@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



Attachment: relpages_stat_update_v1.diff
Description: relpages_stat_update_v1.diff

Reply via email to