On Thu, Dec 5, 2024 at 2:52 PM Pavel Stehule <pavel.steh...@gmail.com> 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".