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-03 07:18:22 |
Message-ID: | CACJufxFwxAvJL944UQKwxV1YuM3GQNTbPRQ6LwtioSKBfpMN2A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-performance |
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>
Attachment | Content-Type | Size |
---|---|---|
v1-0001-refactoring-svariableStartupReceiver.no-cfbot | application/octet-stream | 3.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2025-01-03 09:03:49 | Re: Conflict detection for update_deleted in logical replication |
Previous Message | Shubham Khanna | 2025-01-03 06:54:56 | Re: Log a warning in pg_createsubscriber for max_slot_wal_keep_size |
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2025-01-03 22:59:30 | Re: Re: proposal: schema variables |
Previous Message | Pavel Stehule | 2025-01-02 06:46:06 | Re: Re: proposal: schema variables |