Horton

Re: proposal: schema variables

Laurenz Albe <laurenz.albe@cybertec.at>

View thread
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: 2024-10-25T07:41:40Z
Lists: pgsql-hackers, pgsql-performance

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Move WAL sequence code into its own file

  2. Add ExplainState argument to pg_plan_query() and planner().

  3. Don't include access/htup_details.h in executor/tuptable.h

  4. Refactor to avoid code duplication in transformPLAssignStmt.

  5. Avoid including commands/dbcommands.h in so many places

  6. Restrict psql meta-commands in plain-text dumps.

  7. Split func.sgml into more manageable pieces

  8. Fix squashing algorithm for query texts

  9. EXPLAIN: Always use two fractional digits for row counts.

  10. Preliminary refactoring of plpgsql expression construction.

  11. plpgsql: pure parser and reentrant scanner

  12. Add some sanity checks in executor for query ID reporting

  13. Fix misleading error message context

  14. Add macros for looping through a List without a ListCell.

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