Re: proposal: schema variables

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