Re: Re: proposal: schema variables - Mailing list pgsql-hackers
From | Pavel Stehule |
---|---|
Subject | Re: Re: proposal: schema variables |
Date | |
Msg-id | CAFj8pRBxA868TpLDe9ofXdpVUNmHY8pzkxrjbZ0obCe0g+YZ-Q@mail.gmail.com Whole thread Raw |
In response to | Re: proposal: schema variables (Dmitry Dolgov <9erthalion6@gmail.com>) |
Responses |
Re: Re: proposal: schema variables
|
List | pgsql-hackers |
Hi
pá 27. 12. 2024 v 16:20 odesílatel jian he <jian.universality@gmail.com> napsal:
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);
done
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..)
done
seems IdentifyVariable all these three ereport(ERROR...) don't have
regress tests,
i think we should have it. Am I missing something?
done
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"
changed
I did inspiration from transformAssignmentIndirection now
(2024-12-28 16:07:57) postgres=# let x.a = 20;
ERROR: cannot assign to field "a" of session variable "public.x" because its type integer is not a composite type
LINE 1: let x.a = 20;
^
ERROR: cannot assign to field "a" of session variable "public.x" because its type integer is not a composite type
LINE 1: let x.a = 20;
^
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.
done
+ 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"...)
done
also no coverage tests?
simple test case for it:
create variable v2 as int;
let v2 = '1'::jsonb;
done
---------------<<<>>>--------------
+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($$,"?
let_target is not needed as separate rule now, so I removed it and little bit cleaned (really only little bit)
---------------<<<>>>--------------
i did some refactoring in session_variables.sql for privilege check.
make the tests more neat, please check attached.
merged with minor changes in comment's formatting
I'll send the patch set with next mail
Best regards
Pavel
pgsql-hackers by date: