public inbox for [email protected]  
help / color / mirror / Atom feed
From: 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: proposal: schema variables
Date: Sat, 7 Dec 2024 10:13:26 +0800
Message-ID: <CACJufxHx3FyO4jkv24W9Q_e5weSSy2Nv1Ue437nWU2MQsgu3RA@mail.gmail.com> (raw)
In-Reply-To: <CAFj8pRAGAQf0+0cVVRmUJpwyATtBP1YCRU0SxtVQNg+yT=Ongw@mail.gmail.com>
References: <[email protected]>
	<CAFj8pRAjU-X6rEE9=1++PdtXOPc2uo=yu-tcFXByi-kN3B_7Vw@mail.gmail.com>
	<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>
	<CAFj8pRDy4jhPE8K+Y_0xHqCjWnG65byF+CQFg6Tk+9MwgHYsAw@mail.gmail.com>
	<CAFj8pRA_JetqEqFcZMxhOTBVCHWbvftNipv25OZjhhbe8k4_sw@mail.gmail.com>
	<CAFj8pRCJ1_XZZAWGuFdPjFGf2WgPSbH8LLoFDOTnO4_9Z9o_Ww@mail.gmail.com>
	<CAFj8pRA00BYE25sE9DwrLcsDn6OY5E=FXNSRMCOF34Y=AuP6cw@mail.gmail.com>
	<CAFj8pRAGAQf0+0cVVRmUJpwyATtBP1YCRU0SxtVQNg+yT=Ongw@mail.gmail.com>

On Thu, Dec 5, 2024 at 2:52 PM Pavel Stehule <[email protected]> wrote:
>
> Hi
>
> only rebase

hi.
disclaimer, i *only* applied
v20241205-0001-Enhancing-catalog-for-support-session-variables-and-.patch.

create variable v2 as text;
alter variable v2 rename to v2;
ERROR:  session variable "v2" already exists in schema "public"

the above is  coverage tests for report_namespace_conflict:
        case VariableRelationId:
            Assert(OidIsValid(nspOid));
            msgfmt = gettext_noop("session variable \"%s\" already
exists in schema \"%s\"");
            break;

create type t1 as (a int, b int);
CREATE VARIABLE var1 AS t1;
alter type t1 drop attribute a;
should "alter type t1 drop attribute a;" not allowed?


GetCommandLogLevel also needs to deal with  case T_CreateSessionVarStmt?


there are no regress tests for the change we made in
find_composite_type_dependencies?
It looks like it will be reachable for sure.


CreateVariable, print out error position:
    if (get_typtype(typid) == TYPTYPE_PSEUDO)
        ereport(ERROR,
                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                 errmsg("session variable cannot be pseudo-type %s",
                        format_type_be(typid)),
                 parser_errposition(pstate, stmt->typeName->location)));

Alvaro Herrera told me actually, you don't need the extra parentheses
around errcode.
so you can:
    if (get_typtype(typid) == TYPTYPE_PSEUDO)
        ereport(ERROR,
                errcode(ERRCODE_WRONG_OBJECT_TYPE),
                errmsg("session variable cannot be pseudo-type %s",
                        format_type_be(typid)),
                parser_errposition(pstate, stmt->typeName->location))


pg_variable_is_visible seems to have done twice the system cache call.
maybe follow through with the pg_table_is_visible, pg_type_is_visible
code pattern.


IN src/bin/psql/describe.c
+ appendPQExpBufferStr(&buf, "\nWHERE true\n");
this is not needed?
------------------------------------------------
some of the `foreach` can change to foreach_oid, foreach_node
see: https://git.postgresql.org/cgit/postgresql.git/commit/?id=14dd0f27d7cd56ffae9ecdbe324965073d01a9ff
------------------------------------------------
doc/src/sgml/ref/create_variable.sgml
<programlisting>
CREATE VARIABLE var1 AS date;
LET var1 = current_date;
SELECT var1;
</programlisting>

v20241205-0001-Enhancing-catalog-for-support-session-variables-and-.patch
alone cannot do `LET var1 = current_date;`, `SELECT var1;`
maybe the following patch can do it. but that makes
we cannot commit
v20241205-0001-Enhancing-catalog-for-support-session-variables-and-.patch
alone.
------------------------------------------------
since CREATE VARIABLE is an extension to standard, so in create_schema.sgml
Compatibility section,
do we need to mention CREATE SCHEMA CREATE VARIABLE is an extension
from standard
?
-----------------------------------------------

/*
 * Drop variable by OID, and register the needed session variable
 * cleanup.
 */
void
DropVariableById(Oid varid)
i am not sure of the meaning of "register the needed session variable cleanup".





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: proposal: schema variables
  In-Reply-To: <CACJufxHx3FyO4jkv24W9Q_e5weSSy2Nv1Ue437nWU2MQsgu3RA@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