From: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | Erik Rijkers <er(at)xs4all(dot)nl>, Michael Paquier <michael(at)paquier(dot)xyz>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, DUVAL REMI <REMI(dot)DUVAL(at)cheops(dot)fr>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: proposal: schema variables |
Date: | 2024-10-25 07:41:40 |
Message-ID: | 986faf36db4ebdcbe8397e2c15bb691dbe631ec8.camel@cybertec.at |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-performance |
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
From | Date | Subject | |
---|---|---|---|
Next Message | James Pang | 2024-10-25 07:58:39 | Re: proposal: schema variables |
Previous Message | Kirill Reshke | 2024-10-25 07:31:45 | Re: Useless field ispartitioned in CreateStmtContext |
From | Date | Subject | |
---|---|---|---|
Next Message | James Pang | 2024-10-25 07:58:39 | Re: proposal: schema variables |
Previous Message | Laurenz Albe | 2024-10-25 07:36:45 | Re: lwlock:LockManager wait_events |