Re: Re: proposal: schema variables

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>