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-23T04:11:08Z
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.

Attachments

I have gone over patch 3 from the set and worked on the comments.

Apart from that, I have modified your patch as follows:

> +/*
> + * pg_session_variables - designed for testing
> + *
> + * This is a function designed for testing and debugging.  It returns the
> + * content of sessionvars as-is, and can therefore display entries about
> + * session variables that were dropped but for which this backend didn't
> + * process the shared invalidations yet.
> + */
> +Datum
> +pg_session_variables(PG_FUNCTION_ARGS)
> +{
> +#define NUM_PG_SESSION_VARIABLES_ATTS 8
> +
> +	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.

> +		while ((svar = (SVariable) hash_seq_search(&status)) != NULL)
> +		{
> +			Datum		values[NUM_PG_SESSION_VARIABLES_ATTS];
> +			bool		nulls[NUM_PG_SESSION_VARIABLES_ATTS];
> +			HeapTuple	tp;
> +			bool		var_is_valid = false;
> +
> +			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.

> +			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.

There is a dependency between the variable and the type, but if a concurrent
session drops both the variable and the data type, the non-existing type ID
would still show up in the function output.
Even worse, the OID could have been reused for a different type since.

I am attaching just patch number 3 and leave you to adapt the patch set,
but I don't think any of the other patches should be affected.

Yours,
Laurenz Albe