Re: Schema variables - new implementation for Postgres 15

On 17.03.23 21:50, Pavel Stehule wrote:
> rebase + fix-update pg_dump tests

I have a few comments on the code:

0001

ExecGrant_Variable() could probably use ExecGrant_common().

The additions to syscache.c should be formatted to the new style.

in pg_variable.h:

- create_lsn ought to have a "var" prefix.

- typo: "typmode for variable's type"

- What is the purpose of struct Variable? It seems very similar to
FormData_pg_variable. At least a comment would be useful.

Preserve the trailing comma in ParseExprKind.

0002

expr_kind_allows_session_variables() should have some explanation
about criteria for determining which expression kinds should allow
variables.

Usually, we handle EXPR_KIND_* switches without default case, so we
get notified what needs to be changed if a new enum symbol is added.

0010

The material from the tutorial (advanced.sgml) might be better in
ddl.sgml.

In catalogs.sgml, the columns don't match the ones actually defined in
pg_variable.h in patch 0001 (e.g., create_lsn is missing and the order
doesn't match).

(The order of columns in pg_variable.h didn't immediately make sense to
me either, so maybe there is a middle ground to be found.)

session_variables_ambiguity_warning: There needs to be more
information about this. The current explanation is basically just,
"warn if your query is confusing". Why do I want that? Why would I
not want that? What is the alternative? What are some examples?
Shouldn't there be a standard behavior without a need to configure
anything?

In allfiles.sgml, dropVariable should be before dropView.