On 2023-Feb-06, Peter Smith wrote:
> 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?
Well, here the patch would have us modifying a parse tree node, which is
probably not a good thing to do. I don't remember whether I coded the
deparsing of any other object type this way, but nowadays modifying
parse trees is generally frowned upon. Most likely, this would have to
be done some other way. Maybe set the provider as secondary object
address for the command.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La vida es para el que se aventura"
