public inbox for [email protected]  
help / color / mirror / Atom feed
From: Laurenz Albe <[email protected]>
To: Pavel Stehule <[email protected]>
Cc: Erik Rijkers <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: DUVAL REMI <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: proposal: schema variables
Date: Fri, 25 Oct 2024 10:41:40 +0300
Message-ID: <[email protected]> (raw)
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>
	<[email protected]>
	<CAFj8pRAFktynx5wkanv5SRuzXkZgXu77XpVACiSE=v7i1xHFbw@mail.gmail.com>
	<CAFj8pRA=bn_g5T2AZduy5gNOQoOnUJ+pMHmnRMHi6mR0n=TAsA@mail.gmail.com>
	<[email protected]>
	<CAFj8pRC9de05HSb4tEHDUwJ98+4Wh30W-rJrNOPnTz6ARcv0Fw@mail.gmail.com>
	<CAFj8pRBC5Wz1xHKKBmKsM0xYN0+PdSZ5oXPsk5SZt+VprdUW3A@mail.gmail.com>
	<CAFj8pRAh4pzMoZrKCLt9h+Lr2L=vhgs2PjAF45uLbp_7sijM5w@mail.gmail.com>
	<CAFj8pRA-kxQ1oErcuDeUKYsrgwB5XGLhquatwxOe3dCVy1gcyQ@mail.gmail.com>
	<[email protected]>
	<CAFj8pRBbt2xhY9PyabOY0ZN+Aig6ee3oCon-DM9qi0Uw_3qfbw@mail.gmail.com>
	<CAFj8pRDSa52J7kPmCYXgq1BBbu3YBXwpdSOVpjgU=hnE2k04Cw@mail.gmail.com>
	<CAFj8pRD+QiWOoPrFk2NnPs3t5Eaf4X=aGRV-9ww11cnPP+fV4g@mail.gmail.com>
	<[email protected]>
	<CAFj8pRCGTjqHvH9oeiSf4T6Bydhk9pm033DxxibgF+B7SHC6MQ@mail.gmail.com>
	<CAFj8pRAzNDhFgbZnT0T0mJ7ygA1Qje1Hc0TiKwXM8++kGooPYg@mail.gmail.com>
	<[email protected]>
	<CAFj8pRBarjJYfkN-0-i=JRZJ4PTOYC+K7XgAhfdDqWGqRiPkyw@mail.gmail.com>
	<CAFj8pRDqdWdCULxd5asbKs5C4e9kT2TuKBkR5L-e1=hP5wF2uw@mail.gmail.com>
	<[email protected]>
	<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>
	<[email protected]>
	<CAFj8pRBaD0_bMrCREWnVLfcTMdc0v7ns7Rt=sEvd1EoFmLfarQ@mail.gmail.com>
	<[email protected]>
	<CAFj8pRDXo-RRcy2VFDm_vzv3Eaaz6Ex=X19up=x8W4COyBNmaQ@mail.gmail.com>
	<[email protected]>
	<CAFj8pRDgraz64HHcCoFGCKDM1Cxv1ukELsqfsxux4JP3PM6RyQ@mail.gmail.com>
	<[email protected]>
	<CAFj8pRAsjr0pzQpQeRrQ-wR-ew1oTc6qkW=4355KveN1jKQ+1Q@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAFj8pRDF-F5diNTXr206QfQFauFNK1vu_7NARXd0dtdwGTWLMQ@mail.gmail.com>
	<[email protected]>
	<CAFj8pRCy8NTG3ewOrH0KLQgrMpVMSzPjhoLKW=c1Jjz8QWNSKw@mail.gmail.com>
	<CAFj8pRB-ey7mCnKqMhBBrkv2mXOvTLfAEBy2LZxQsWPCBvguGQ@mail.gmail.com>
	<[email protected]>
	<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>
	<[email protected]>
	<CAFj8pRAxSiA68tS=ky64A9fhKGstRVpgQfPotNmMF0-4GuisRQ@mail.gmail.com>

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

Thanks.

> > > +                     memset(values, 0, sizeof(values));
> > > +                     memset(nulls, 0, sizeof(nulls));
> > 
> > Instead of explicitly zeroing out the arrays, I have used an empty initializer
> > in the definition, like
> > 
> >   bool          nulls[NUM_PG_SESSION_VARIABLES_ATTS] = {};
> > 
> > That should have the same effect.
> > If you don't like that, I have no real problem with your original code.
> 
> I prefer the original way - minimally it is a common pattern. I didn't find any usage of `= {} ` in code

That's alright by me.


> > > +                     values[0] = ObjectIdGetDatum(svar->varid);
> > > +                     values[3] = ObjectIdGetDatum(svar->typid);
> > 
> > You are using the type ID without checking if it exists in the catalog.
> > I think that is a bug.
> 
> The original idea was using typid as hint identification of deleted variables. The possibility
> that this id will not be consistent for the current catalogue was expected. 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.

> 
> owing typid has some information value, but I don't think it is absolutely necessary. I see some possible changes:
> 
> 1. no change
> 2. remove typid column
> 3. show typid only when variable is valid, and using regtype as output type, remove typname
> 
> 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





view thread (439+ messages)  latest in thread

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: proposal: schema variables
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox