Message-ID: <986faf36db4ebdcbe8397e2c15bb691dbe631ec8.camel@cybertec.at>
Subject: Re: proposal: schema variables
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Erik Rijkers <er@xs4all.nl>, Michael Paquier <michael@paquier.xyz>, Amit
 Kapila <amit.kapila16@gmail.com>, DUVAL REMI <REMI.DUVAL@cheops.fr>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: Fri, 25 Oct 2024 10:41:40 +0300
In-Reply-To: 
 <CAFj8pRAxSiA68tS=ky64A9fhKGstRVpgQfPotNmMF0-4GuisRQ@mail.gmail.com>
References: 
	<CAFj8pRBoGBKopkiTa4ki3dMgy-cSTRZ-BQPKOq7=Tk0SSNsowA@mail.gmail.com>
	 <CAFj8pRBCiWn12H9FHymYs17fk68nRd9Xpn+SYf18TLdb2YSUrQ@mail.gmail.com>
	 <CAA4eK1JV-Ox0oRFdXnPqXSzM84wTR5QFkRzCpNVF_+0FNjS5Mg@mail.gmail.com>
	 <CAFj8pRByCCcgDkeXyafAnH6LgxtAyCVwg6yfJAhyTY6GLscfZg@mail.gmail.com>
	 <CAFj8pRBvUonC_ug3F=w1Q55Dp=DggojvAeL7Vmh14Q-WhFHxzw@mail.gmail.com>
	 <CAFj8pRDj72P-f=SUygtOXnTOBQ0RzmL_fN=wLfaCzcbPVpGgzw@mail.gmail.com>
	 <CAFj8pRDD2GQaJ_iDT4vSVe658+oHRXU2S2af7Y1-it9jaP8VFg@mail.gmail.com>
	 <20200924035637.GF28585@paquier.xyz>
	 <CAFj8pRAFktynx5wkanv5SRuzXkZgXu77XpVACiSE=v7i1xHFbw@mail.gmail.com>
	 <CAFj8pRA=bn_g5T2AZduy5gNOQoOnUJ+pMHmnRMHi6mR0n=TAsA@mail.gmail.com>
	 <20201001033824.GC8130@paquier.xyz>
	 <CAFj8pRC9de05HSb4tEHDUwJ98+4Wh30W-rJrNOPnTz6ARcv0Fw@mail.gmail.com>
	 <CAFj8pRBC5Wz1xHKKBmKsM0xYN0+PdSZ5oXPsk5SZt+VprdUW3A@mail.gmail.com>
	 <CAFj8pRAh4pzMoZrKCLt9h+Lr2L=vhgs2PjAF45uLbp_7sijM5w@mail.gmail.com>
	 <CAFj8pRA-kxQ1oErcuDeUKYsrgwB5XGLhquatwxOe3dCVy1gcyQ@mail.gmail.com>
	 <51a9a68e8a998d04df17417d45c1dbd4@xs4all.nl>
	 <CAFj8pRBbt2xhY9PyabOY0ZN+Aig6ee3oCon-DM9qi0Uw_3qfbw@mail.gmail.com>
	 <CAFj8pRDSa52J7kPmCYXgq1BBbu3YBXwpdSOVpjgU=hnE2k04Cw@mail.gmail.com>
	 <CAFj8pRD+QiWOoPrFk2NnPs3t5Eaf4X=aGRV-9ww11cnPP+fV4g@mail.gmail.com>
	 <bdeb7611c6dbf22226c302bcc01d178b@xs4all.nl>
	 <CAFj8pRCGTjqHvH9oeiSf4T6Bydhk9pm033DxxibgF+B7SHC6MQ@mail.gmail.com>
	 <CAFj8pRAzNDhFgbZnT0T0mJ7ygA1Qje1Hc0TiKwXM8++kGooPYg@mail.gmail.com>
	 <89817942c99da01cd5e7850fe418436b@xs4all.nl>
	 <CAFj8pRBarjJYfkN-0-i=JRZJ4PTOYC+K7XgAhfdDqWGqRiPkyw@mail.gmail.com>
	 <CAFj8pRDqdWdCULxd5asbKs5C4e9kT2TuKBkR5L-e1=hP5wF2uw@mail.gmail.com>
	 <56ca532c37eb0b540961f74a7bd5db39@xs4all.nl>
	 <CAFj8pRCPW56pFr0F0BcasdXjFeo3SFixNSpWKaBk0ibvznum-A@mail.gmail.com>
	 <CAFj8pRD1Feit93CgwmYm1Q=X+M+AZqffCEZPFQ7qEMNHZRN4fA@mail.gmail.com>
	 <CAFj8pRCc=B9-FRQg5eWDSkGwS2vpkq88hR6042cmPPizHuEGSA@mail.gmail.com>
	 <CAFj8pRBk8x7afUXKLBOU-Ctg6A7QJvTAGGVEi0b6Jc8YTe8nUg@mail.gmail.com>
	 <CAFj8pRCSwHQ4BJUbjF2YEausK1Z6+ejMyedpqAnWJbG+FEJDLw@mail.gmail.com>
	 <CAFj8pRAbY+N+UqjqgESL5x-bsGmV+aVyyUkxUSgaGDZToZjDqQ@mail.gmail.com>
	 <CAFj8pRBzKcqzj=23BHfv1QaXHt=2_SN=uhdR3rb_dAVQoit7ug@mail.gmail.com>
	 <CAFj8pRCi-n6SzkAB+OHG=TZvL13xxta_qgffBLDOY0HEBqDhvg@mail.gmail.com>
	 <8181bd3abc647bdae5a4f78e71e62478a98c75f4.camel@cybertec.at>
	 <CAFj8pRBaD0_bMrCREWnVLfcTMdc0v7ns7Rt=sEvd1EoFmLfarQ@mail.gmail.com>
	 <9e67d49deb18270eddb95e602c83f02b98459843.camel@cybertec.at>
	 <CAFj8pRDXo-RRcy2VFDm_vzv3Eaaz6Ex=X19up=x8W4COyBNmaQ@mail.gmail.com>
	 <f9f4d536993e48990bd7eb38e18798c4572b6700.camel@cybertec.at>
	 <CAFj8pRDgraz64HHcCoFGCKDM1Cxv1ukELsqfsxux4JP3PM6RyQ@mail.gmail.com>
	 <3b662dc5b615d4c20a55e8e2fbe6fc00fe00609d.camel@cybertec.at>
	 <CAFj8pRAsjr0pzQpQeRrQ-wR-ew1oTc6qkW=4355KveN1jKQ+1Q@mail.gmail.com>
	 <c29fb07fb1b55725e175e5625a5261baeab4a977.camel@cybertec.at>
	 <6996931e8c9edf3b82223e74e92326a7ed06c1d6.camel@cybertec.at>
	 <67aa68a7e6dfb44c0cbbdf7f97cadfede4269ce5.camel@cybertec.at>
	 <CAFj8pRDF-F5diNTXr206QfQFauFNK1vu_7NARXd0dtdwGTWLMQ@mail.gmail.com>
	 <04ec666686e9e21cb515617df06885c66f3d34ce.camel@cybertec.at>
	 <CAFj8pRCy8NTG3ewOrH0KLQgrMpVMSzPjhoLKW=c1Jjz8QWNSKw@mail.gmail.com>
	 <CAFj8pRB-ey7mCnKqMhBBrkv2mXOvTLfAEBy2LZxQsWPCBvguGQ@mail.gmail.com>
	 <3850a85012d040827b10193189edbe2c23a64f8f.camel@cybertec.at>
	 <CAFj8pRBvUwfs96MX2_=unHQ107diUOK4S_WD=E4XBXkpXG3VjA@mail.gmail.com>
	 <CAFj8pRCDWjgo=yLEK-jRDe5VwuNSQSmMqPPmysQf9nHA8gifnw@mail.gmail.com>
	 <CAFj8pRA2jbXRTGgM1co8gLCNttgaH8miRUSSauMJKWgousDsDg@mail.gmail.com>
	 <CAFj8pRA6nJRg8oq5k_Cdk5oH5_FjtViG8XLmHKrkqOuiMT_CKQ@mail.gmail.com>
	 <CAFj8pRBWMLP3Vyr8z+19eaiJKQoVtBfmDhNJFKXDX6uFzd4vBQ@mail.gmail.com>
	 <c7eee33c5051f973ab256ee7202c15771c33f47d.camel@cybertec.at>
	 <CAFj8pRAxSiA68tS=ky64A9fhKGstRVpgQfPotNmMF0-4GuisRQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.4 (3.52.4-2.fc40) 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/986faf36db4ebdcbe8397e2c15bb691dbe631ec8.camel%40cybertec.at>
Precedence: bulk

On Fri, 2024-10-25 at 07:21 +0200, Pavel Stehule wrote:
> > > +=C2=A0 =C2=A0 =C2=A0elog(DEBUG1, "pg_session_variables start");
> >=20
> > I don't think that message is necessary, particularly with DEBUG1.
> > I have removed this message and the "end" message as well.
>=20
> removed

Thanks.

> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0memset(values, 0, sizeof(values));
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0memset(nulls, 0, sizeof(nulls));
> >=20
> > Instead of explicitly zeroing out the arrays, I have used an empty init=
ializer
> > in the definition, like
> >=20
> > =C2=A0 bool=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nulls[NUM_PG_SESSION_VARI=
ABLES_ATTS] =3D {};
> >=20
> > That should have the same effect.
> > If you don't like that, I have no real problem with your original code.
>=20
> I prefer the original way - minimally it is a common pattern. I didn't fi=
nd any usage of `=3D {} ` in code

That's alright by me.


> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0values[0] =3D ObjectIdGetDatum(svar->varid);
> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0values[3] =3D ObjectIdGetDatum(svar->typid);
> >=20
> > You are using the type ID without checking if it exists in the catalog.
> > I think that is a bug.
>=20
> The original idea was using typid as hint identification of deleted varia=
bles. The possibility
> that this id will not be consistent for the current catalogue was expecte=
d. And it
> is a reason why the result type is just Oid and not regtype. Without it, =
pg_session_variables
> shows just empty rows (except oid) for dropped not yet purged variables.

I see your point.  It is for testing and debugging only.

>=20
> owing typid has some information value, but I don't think it is absolutel=
y necessary. I see some possible changes:
>=20
> 1. no change
> 2. remove typid column
> 3. show typid only when variable is valid, and using regtype as output ty=
pe, remove typname
>=20
> What do you prefer?

I'd say leave it as it is.  I agree that it is not dangerous, and if it is =
intentional that
non-existing type IDs might be displayed, I have no problem with it.

Yours,
Laurenz Albe