Re: Re: proposal: schema variables - Mailing list pgsql-hackers

From jian he
Subject Re: Re: proposal: schema variables
Date
Msg-id CACJufxHvmagrYNO8rjAd85c8aHzs3cZpiPBiyBjNDUK8GaA6Zg@mail.gmail.com
Whole thread Raw
In response to Re: proposal: schema variables  (Dmitry Dolgov <9erthalion6@gmail.com>)
List pgsql-hackers
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.



pgsql-hackers by date:

Previous
From: Bertrand Drouvot
Date:
Subject: Re: per backend I/O statistics
Next
From: "Zhijie Hou (Fujitsu)"
Date:
Subject: RE: Conflict detection for update_deleted in logical replication