Horton

Re: Re: proposal: schema variables

Pavel Stehule <pavel.stehule@gmail.com>

View thread
From: Pavel Stehule <pavel.stehule@gmail.com>
To: jian he <jian.universality@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>
Date: 2025-01-03T22:59:30Z
Lists: pgsql-hackers, pgsql-performance

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Move WAL sequence code into its own file

  2. Add ExplainState argument to pg_plan_query() and planner().

  3. Don't include access/htup_details.h in executor/tuptable.h

  4. Refactor to avoid code duplication in transformPLAssignStmt.

  5. Avoid including commands/dbcommands.h in so many places

  6. Restrict psql meta-commands in plain-text dumps.

  7. Split func.sgml into more manageable pieces

  8. Fix squashing algorithm for query texts

  9. EXPLAIN: Always use two fractional digits for row counts.

  10. Preliminary refactoring of plpgsql expression construction.

  11. plpgsql: pure parser and reentrant scanner

  12. Add some sanity checks in executor for query ID reporting

  13. Fix misleading error message context

  14. Add macros for looping through a List without a ListCell.

Attachments

Hi

pá 3. 1. 2025 v 8:18 odesílatel jian he <jian.universality@gmail.com>
napsal:

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

It is true. The syntax allows just `expression` or `(subquery)`, and  all
dirty work does execution of subquery

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

done + regress tests


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

changed


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

I think the sentence  "and is reset to default value (null) when the
session ends" is a little bit misguided. The memory is
really released. I changed it to

     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.
     The default value of the session variable is null.  Read or write
access
     to session variables is controlled by privileges, similar to other
database
     objects.



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

done

Regards

Pavel