MIME-Version: 1.0
References: 
 <CAFj8pRC+hPCc2X88xC=pTJoqmVPApDsageZOMyqaxi5788WxHA@mail.gmail.com>
 <CAFj8pRDJ9cq00VYSHxs6LsoHNWjhYXyWWBtV6UgeWwhs0AHa9A@mail.gmail.com>
 <CAFj8pRBPXTcw_3fpKtgVthV2+9rZGhxitZ40DnAwCrK601TZZg@mail.gmail.com>
 <ndtfl4tsnpkb7m7hwvnmlpsascpgd3a7xvjmjhtxffsbrgygtm@4du6zsmnnwq5>
 <CAFj8pRAu4XvNCGu1751t=2YEqLqTjDA3FavMExm2S0KYQq=DdQ@mail.gmail.com>
 <CAFj8pRAsEoeZv0HEnA8CKgFKDSQ-wYw18Os1vdksWCV7ez2bVw@mail.gmail.com>
 <3chredgnjcmccym2kczawfih226b4ac6co7p6z4jeofevrcosi@mrsxkx2x2c65>
 <CAFj8pRBoWPDTOwn5FmMzc+1qiopw+N04U26nviOdF61fs8A2wQ@mail.gmail.com>
 <stckyvkl4yyzvgjsaawojs3xikke7mmds5bhv7l7qerclywywk@h4v4n43xm6u2>
 <CAFj8pRB_E1GM_YGT-ti4bXka6mhLdAAFeTe+BHgHFYC+qb-76g@mail.gmail.com>
 <20241120201313.t4wbhld4ktgielaf@erthalion.local>
 <CAFj8pRBWqEb8i6WmrF_Xh64=48GtisKijgczMv7HTTpe4GswuA@mail.gmail.com>
 <CAFj8pRAry0esQiHcK=6BwwFKDY0zanug6k07CEQzRPBqZ6iW0Q@mail.gmail.com>
 <CACJufxFNjKrmyEi9SLfPCq4c9GUN+5eoOtbZwBPq9eKoO8REUw@mail.gmail.com>
 <CAFj8pRALQ-j-Dz3R1ivCoXut8LEhN+kSa7U8Gshucdv5zU3AfQ@mail.gmail.com>
 <CACJufxG7LvaNbF8ZSCcxOVUbm9W=KGjD=h_Wk+5imMw4s_2QxA@mail.gmail.com>
 <CAFj8pRBY_2awVdER5piyy_JPqsU1Sgr4HLO-v6C1nUS3dJnang@mail.gmail.com>
 <CACJufxEM=BLEn6YfgGonM7yuXMn7iqQJcH5PnDVbajWKanynfg@mail.gmail.com>
 <CAFj8pRBxA868TpLDe9ofXdpVUNmHY8pzkxrjbZ0obCe0g+YZ-Q@mail.gmail.com>
 <CACJufxEb1prZo4G4BhYym1VqKHaPVUv3SoggTR4uCqgCRY7Q-Q@mail.gmail.com>
 <CAFj8pRDK2heEhQNcwD=tcKpG5YyPu7zOz2jdRW5A5QfoSUEi_g@mail.gmail.com>
 <CACJufxEk16ArmFuTV9VYi6DJaZ+9Jrbt0-u0Zuov7sk-vpg_Dg@mail.gmail.com>
 <CAFj8pRAaoPCc=n_TPpWdOvzEdc96xfJZR8W7PphMLxLbGxAq-Q@mail.gmail.com>
 <CAFj8pRAzFCbTwqjaEvkvB0VBa9ofSEYxnv8wM3pHZTGtrRK0yQ@mail.gmail.com>
 <CACJufxFwxAvJL944UQKwxV1YuM3GQNTbPRQ6LwtioSKBfpMN2A@mail.gmail.com>
 <CAFj8pRAVoWzkx8Y3or-frYwszyLSjxzs9jYTm3rx00qN+21ZHg@mail.gmail.com>
 <CAFj8pRDhpDwWrV7DbwoGz=-i3yesEU1oooYyMErQfjMXKw=pOQ@mail.gmail.com>
 <CACJufxGdRAF=e57W3yd1cTrspB2bucnsr6ZfYY=bs+bfJWdRPw@mail.gmail.com>
 <CACJufxG6tFo1uOtTPoUw0ntzHddahd7EmR37DhsxMXxUixPaeQ@mail.gmail.com>
 <CACJufxG7Mc2PK2B7+vkdDxvrsJNN3r+cpzvAFTZZXKnWZOcG6w@mail.gmail.com>
 <CAFj8pRAF8W6Sn91UH9gPtOV4ss99D0et+yvnQf=v0n=85KhWXQ@mail.gmail.com>
 <CAFj8pRD67R73TcbN9sShbKLdtoLGRsM8GJVC_OqY=EWe3sJ1xw@mail.gmail.com>
In-Reply-To: 
 <CAFj8pRD67R73TcbN9sShbKLdtoLGRsM8GJVC_OqY=EWe3sJ1xw@mail.gmail.com>
From: jian he <jian.universality@gmail.com>
Date: Tue, 7 Jan 2025 17:07:22 +0800
Message-ID: 
 <CACJufxHvmagrYNO8rjAd85c8aHzs3cZpiPBiyBjNDUK8GaA6Zg@mail.gmail.com>
Subject: Re: Re: proposal: schema variables
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>,
 Laurenz Albe <laurenz.albe@cybertec.at>,
	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>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CACJufxHvmagrYNO8rjAd85c8aHzs3cZpiPBiyBjNDUK8GaA6Zg%40mail.gmail.com>
Precedence: bulk

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.