On Fri, Feb 3, 2023 at 9:21 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
>
> On 2023-Feb-03, Peter Smith wrote:
>
...
> > 3. ExecuteGrantStmt
> >
> > + /* Copy the grantor id needed for DDL deparsing of Grant */
> > + istmt.grantor_uid = grantor;
> > +
> >
> > SUGGESTION (comment)
> > Copy the grantor id to the parsetree, needed for DDL deparsing of Grant
>
> Is istmt really "the parse tree" actually? As I recall, it's a derived
> struct that's created during execution of the grant/revoke command, so
> modifying the comment like this would be a mistake.
>
I thought this comment was analogous to another one from this same
patch 0001 (see seclabel.c), so the suggested change above was simply
to make the wording consistent.
@@ -134,6 +134,9 @@ ExecSecLabelStmt(SecLabelStmt *stmt)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("must specify provider when multiple security label providers
have been loaded")));
provider = (LabelProvider *) linitial(label_provider_list);
+
+ /* Copy the provider name to the parsetree, needed for DDL deparsing
of SecLabelStmt */
+ stmt->provider = pstrdup(provider->provider_name);
So if the suggestion for the ExecuteGrantStmt comment was a mistake
then perhaps the ExecSecLabelStmt comment is wrong also?
------
Kind Regards,
Peter Smith.
Fujitsu Australia
