Horton

Re: Re: proposal: schema variables

jian he <jian.universality@gmail.com>

View thread
From: jian he <jian.universality@gmail.com>
To: Pavel Stehule <pavel.stehule@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: 2024-12-27T15:19:29Z
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.

+ if (!OidIsValid(varid))
+ AcceptInvalidationMessages();
+ else if (OidIsValid(varid))
+ LockDatabaseObject(VariableRelationId, varid, 0, AccessShareLock);

we can change it to
+ if (!OidIsValid(varid))
+ AcceptInvalidationMessages();
+ else
+ LockDatabaseObject(VariableRelationId, varid, 0, AccessShareLock);

inval_count didn't explain at all, other places did actually explain it.
Can we add some text explaining inval_count? (i didn't fully understand
this part, that is why i am asking..)

seems IdentifyVariable all these three ereport(ERROR...) don't have
regress tests,
i think we should have it. Am I missing something?

create variable v2 as int;
let v2.a = 1;
ERROR:  type "integer" of target session variable "public.v2" is not a
composite type
LINE 1: let v2.a = 1;
            ^
the error messages look weird.
IMO, it should either be
"type of session variable "public.v2" is not a composite type"
or
"session variable "public.v2" don't have attribute "a"

also, the above can be as a regress test for:
+ if (attrname && !is_rowtype)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("type \"%s\" of target session variable \"%s.%s\" is not a
composite type",
+ format_type_be(typid),
+ get_namespace_name(get_session_variable_namespace(varid)),
+ get_session_variable_name(varid)),
+ parser_errposition(pstate, stmt->location)));
since we don't have coverage tests for it.


+ if (coerced_expr == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("variable \"%s.%s\" is of type %s,"
+ " but expression is of type %s",
+ get_namespace_name(get_session_variable_namespace(varid)),
+ get_session_variable_name(varid),
+ format_type_be(typid),
+ format_type_be(exprtypid)),
+ errhint("You will need to rewrite or cast the expression."),
+ parser_errposition(pstate, exprLocation((Node *) expr))));

generally, errmsg(...) should put it into one line for better grep-ability
so we can change it to:
+errmsg("variable \"%s.%s\" is of type %s, but expression is of type %s"...)

also no coverage tests?
simple test case for it:
create variable v2 as int;
let v2 = '1'::jsonb;

---------------<<<>>>--------------
+let_target:
+ ColId opt_indirection
+ {
+ $$ = list_make1(makeString($1));
+ if ($2)
+  $$ = list_concat($$,
+   check_indirection($2, yyscanner));
+ }
if you look closely, it seems the indentation level is wrong in
line "$$ = list_concat($$,"?

---------------<<<>>>--------------
i did some refactoring in session_variables.sql for privilege check.
make the tests more neat, please check attached.