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-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

In response to

Responses

Browse pgsql-hackers by date

  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

Browse pgsql-performance by date

  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