public inbox for [email protected]
help / color / mirror / Atom feedFrom: jian he <[email protected]>
To: Pavel Stehule <[email protected]>
Cc: Dmitry Dolgov <[email protected]>
Cc: Laurenz Albe <[email protected]>
Cc: Erik Rijkers <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: DUVAL REMI <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Re: proposal: schema variables
Date: Fri, 3 Jan 2025 15:18:22 +0800
Message-ID: <CACJufxFwxAvJL944UQKwxV1YuM3GQNTbPRQ6LwtioSKBfpMN2A@mail.gmail.com> (raw)
In-Reply-To: <CAFj8pRAzFCbTwqjaEvkvB0VBa9ofSEYxnv8wM3pHZTGtrRK0yQ@mail.gmail.com>
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>
<[email protected]>
<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>
hi.
in the function svariableStartupReceiver all these "ereport(ERROR"
cannot happen,
since transformLetStmt already did all the heavy work.
base on https://www.postgresql.org/docs/current/error-message-reporting.html
all these "ereport(ERROR," in the svariableStartupReceiver can be
simplified as "elog(ERROR,"
or Assert.
After standard_ExecutorStart->InitPlan, queryDesc.tupDesc will not
include attr->attisdropped is true scarenio.
In standard_ExecutorStart, I added the following code then ran the
regress test again to prove my point.
standard_ExecutorStart
/*
* Initialize the plan state tree
*/
InitPlan(queryDesc, eflags);
for (int i = 0; i < queryDesc->tupDesc->natts; i++)
{
Form_pg_attribute attr = TupleDescAttr(queryDesc->tupDesc, i);
if (attr->attisdropped)
{
elog(INFO, "some attribute is dropped queryDesc->operation
is %d", queryDesc->operation);
}
}
MemoryContextSwitchTo(oldcontext);
-------------------------
svariableStartupReceiver parameter typeinfo is from queryDesc->tupDesc
So I think svariableStartupReceiver, typeinfo->natts will always equal one.
therefore SVariableState.slot_offset is not necessary.
overall, i think svariableStartupReceiver can be simplified as the following:
static void
svariableStartupReceiver(DestReceiver *self, int operation, TupleDesc typeinfo)
{
SVariableState *myState = (SVariableState *) self;
int natts = typeinfo->natts;
Form_pg_attribute attr;
LOCKTAG locktag PG_USED_FOR_ASSERTS_ONLY;
Assert(myState->pub.mydest == DestVariable);
Assert(OidIsValid(myState->varid));
Assert(SearchSysCacheExists1(VARIABLEOID, myState->varid));
#ifdef USE_ASSERT_CHECKING
SET_LOCKTAG_OBJECT(locktag,
MyDatabaseId,
VariableRelationId,
myState->varid,
0);
Assert(LockHeldByMe(&locktag, AccessShareLock, false));
#endif
Assert(natts == 1);
attr = TupleDescAttr(typeinfo, 0);
myState->need_detoast = attr->attlen == -1;
myState->rows = 0;
}
I've attached the file containing the changes I mentioned earlier.
-------------------------<<>>>-------------------------------
Overall, 0001 and 0002 the doc looks good to me now.
The following are only some minor issues I came up with.
In Table 5.1. ACL Privilege Abbreviations
<table id="privilege-abbrevs-table">
<title><acronym>ACL</acronym> Privilege Abbreviations</title>
<literal>VARIABLE</literal> (occurred 3 times)
should be
<literal>SESSION VARIABLE</literal>
?
doc/src/sgml/glossary.sgml
I want to do minor tweak. from
<para>
A persistent database object that holds a value in session memory. This
value is private to each session and is released when the session ends.
Read or write access to session variables is controlled by privileges,
similar to other database objects.
</para>
to
<para>
A persistent database object that holds a value in session memory. This
value is private to each session and is reset to default value
(null) when the session ends.
Read or write access to session variables is controlled by access
privileges,
similar to other database objects.
</para>
in let.sgml.
<para>
Example:
<programlisting>
CREATE VARIABLE myvar AS integer;
LET myvar = 10;
LET myvar = (SELECT sum(val) FROM tab);
</programlisting>
</para>
it should be
<refsect1>
<title>Examples</title>
...your example code
</refsect1>
Attachments:
[application/octet-stream] v1-0001-refactoring-svariableStartupReceiver.no-cfbot (3.8K, 2-v1-0001-refactoring-svariableStartupReceiver.no-cfbot)
download
view thread (439+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Re: proposal: schema variables
In-Reply-To: <CACJufxFwxAvJL944UQKwxV1YuM3GQNTbPRQ6LwtioSKBfpMN2A@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox