Re: Re: proposal: schema variables

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, 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: Re: proposal: schema variables
Date: 2025-01-07 09:07:22
Message-ID: CACJufxHvmagrYNO8rjAd85c8aHzs3cZpiPBiyBjNDUK8GaA6Zg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-performance

hi.

some minor issues.
'transformMergeStmt also needs
"qry->hasSessionVariables = pstate->p_hasSessionVariables;"
?

<function>acldefault</function> in doc/src/sgml/func.sgml
Need an update for SESSION VARIABLE?

Table 9.70. Access Privilege Inquiry Functions
sorting order: has_session_variable_privilege should after has_server_privilege?

Similar to src/test/regress/sql/event_trigger.sql,
we can use the following query to test four functions in
Table 9.79. Object Information and Addressing Functions
(9.27.5. Object Information and Addressing Functions)
for session variables.

SELECT
e.varname,
pg_describe_object('pg_variable'::regclass, e.oid, 0) as descr,
b.type, b.object_names, b.object_args,
pg_identify_object(a.classid, a.objid, a.objsubid) as ident
FROM pg_variable as e,
LATERAL pg_identify_object_as_address('pg_variable'::regclass, e.oid, 0) as b,
LATERAL pg_get_object_address(b.type, b.object_names, b.object_args) as a;

in function transformColumnRef
if (expr_kind_allows_session_variables(pstate->p_expr_kind))
{
}
can change to
if (!node &&
!(relname && crerr == CRERR_NO_COLUMN) &&
expr_kind_allows_session_variables(pstate->p_expr_kind))
{
}
can reduce the function call of expr_kind_allows_session_variables,
also not lose readability.

typedef struct SVariableData says:
/*
* Stored value and type description can be outdated when we receive a
* sinval message. We then have to check if the stored data are still
* trustworthy.
*/
bool is_valid;

CREATE VARIABLE var3 AS int;
LET var3 = 10;
GRANT SELECT ON VARIABLE var3 TO regress_noowner;
I don't understand why the last statement GRANT SELECT needs to call
pg_variable_cache_callback?
and it will invalidate the original var3.

+/*
+ * Returns a copy of the value of the session variable (in the current memory
+ * context). The caller is responsible for permission checks.
+ */
+Datum
+GetSessionVariable(Oid varid, bool *isNull, Oid *typid)
+{
+ SVariable svar;
+
+ svar = get_session_variable(varid);
+
+ /*
+ * Although "svar" is freshly validated in this point, svar->is_valid can
+ * be false, if an invalidation message ws processed during the domain check.
+ * But the variable and all its dependencies are locked now, so we don't need
+ * to repeat the validation.
+ */
+ Assert(svar);
+
+ *typid = svar->typid;
+
+ return copy_session_variable_value(svar, isNull);
+}
typo, "ws processed" should be "was processed".
also the Assert is not helpful? since svar is NULL,
get_session_variable would have segfault.
also SVariable svar; is not initialized to NULL, so generally it will
not be NULL by default.
also the comments seem to imply that before
copy_session_variable_value, svar->is_valid can be false.
if svar->is_valid is false, then why do copy_session_variable_value.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhijie Hou (Fujitsu) 2025-01-07 09:10:49 RE: Conflict detection for update_deleted in logical replication
Previous Message Bertrand Drouvot 2025-01-07 08:54:59 Re: per backend I/O statistics

Browse pgsql-performance by date

  From Date Subject
Next Message Jeff Janes 2025-01-07 19:30:33 Re: Aggressive vacuum
Previous Message Slava Mudry 2025-01-06 23:18:28 Re: Aggressive vacuum