public inbox for [email protected]
help / color / mirror / Atom feedFrom: Pavel Stehule <[email protected]>
To: jian he <[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: Re: proposal: schema variables
Date: Sat, 28 Dec 2024 16:32:17 +0100
Message-ID: <CAFj8pRBxA868TpLDe9ofXdpVUNmHY8pzkxrjbZ0obCe0g+YZ-Q@mail.gmail.com> (raw)
In-Reply-To: <CACJufxEM=BLEn6YfgGonM7yuXMn7iqQJcH5PnDVbajWKanynfg@mail.gmail.com>
References: <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>
<[email protected]>
<CAFj8pRBWqEb8i6WmrF_Xh64=48GtisKijgczMv7HTTpe4GswuA@mail.gmail.com>
<CAFj8pRAry0esQiHcK=6BwwFKDY0zanug6k07CEQzRPBqZ6iW0Q@mail.gmail.com>
<CACJufxFNjKrmyEi9SLfPCq4c9GUN+5eoOtbZwBPq9eKoO8REUw@mail.gmail.com>
<CAFj8pRALQ-j-Dz3R1ivCoXut8LEhN+kSa7U8Gshucdv5zU3AfQ@mail.gmail.com>
<CACJufxG7LvaNbF8ZSCcxOVUbm9W=KGjD=h_Wk+5imMw4s_2QxA@mail.gmail.com>
<CAFj8pRBY_2awVdER5piyy_JPqsU1Sgr4HLO-v6C1nUS3dJnang@mail.gmail.com>
<CACJufxEM=BLEn6YfgGonM7yuXMn7iqQJcH5PnDVbajWKanynfg@mail.gmail.com>
Hi
pá 27. 12. 2024 v 16:20 odesílatel jian he <[email protected]>
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;
^
>
> 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
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: Re: proposal: schema variables
In-Reply-To: <CAFj8pRBxA868TpLDe9ofXdpVUNmHY8pzkxrjbZ0obCe0g+YZ-Q@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