pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Clarify deleting comments and security labels in synopsis
Date: Tue, 31 Jan 2023 18:07:15 +0000
Message-ID: <87pmauk27g.fsf@wibble.ilmari.org> (raw)
In-Reply-To: <1139370.1675186526@sss.pgh.pa.us>
References: <87sffqk4zp.fsf@wibble.ilmari.org>
	<1139370.1675186526@sss.pgh.pa.us>

Tom Lane <tgl@sss.pgh.pa.us> writes:

> =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org> writes:
>> A user on IRC was confused about how to delete a security label using
>> the `SECURITY LABLEL ON … IS …` command, and looking at the docs I can
>> see why.
>
>> The synopsis just says `IS 'label'`, which implies that it can only be a
>> string. It's not until you read the description for `label` that you
>> see "or `NULL` to drop the security label."  I propose making the
>> synopsis say `IS { 'label' | NULL }` to make it clear that it can be
>> NULL as well.  The same applies to `COMMENT ON … IS …`, which I've also
>> changed similarly in the attached.
>
> Agreed; as-is, the syntax summary is not just confusing but outright
> wrong.
>
> I think we could go further and split the entry under Parameters
> to match:
>
> 	'text'
> 		The new comment (must be a simple string literal,
> 		not an expression).
>
> 	NULL
> 		Write NULL to drop the comment.

Makes sense. Something like the attached v2?

> 			regards, tom lane

- ilmari

Attachments:

v2-0001-Clarify-that-COMMENT-and-SECURITY-LABEL-can-be-se.patch (text/x-diff, 3.3K) download From 0fb27fc8a5f484b87df6068076987719f0fb79bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org> Date: Tue, 31 Jan 2023 16:05:20 +0000 Subject: [PATCH v2] Clarify that COMMENT and SECURITY LABEL can be set to NULL in the synopses This was only mentioned in the description of the text/label, which are marked as being in quotes in the synopsis, which can cause confusion (as witnessed on IRC). Also separate the literal and NULL case in the parameter list, per suggestion from Tom Lane. --- doc/src/sgml/ref/comment.sgml | 16 ++++++++++++---- doc/src/sgml/ref/security_label.sgml | 16 ++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml index 7499da1d62..470a6d0b5c 100644 --- a/doc/src/sgml/ref/comment.sgml +++ b/doc/src/sgml/ref/comment.sgml @@ -66,7 +66,7 @@ TRIGGER <replaceable class="parameter">trigger_name</replaceable> ON <replaceable class="parameter">table_name</replaceable> | TYPE <replaceable class="parameter">object_name</replaceable> | VIEW <replaceable class="parameter">object_name</replaceable> -} IS '<replaceable class="parameter">text</replaceable>' +} IS { '<replaceable class="parameter">text</replaceable>' | NULL } <phrase>where <replaceable>aggregate_signature</replaceable> is:</phrase> @@ -263,11 +263,19 @@ </varlistentry> <varlistentry> - <term><replaceable class="parameter">text</replaceable></term> + <term><literal>'</literal><replaceable class="parameter">text</replaceable><literal>'</literal></term> <listitem> <para> - The new comment, written as a string literal; or <literal>NULL</literal> - to drop the comment. + The new comment, written as a string literal. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>NULL</literal></term> + <listitem> + <para> + Write <literal>NULL</literal> to drop the comment. </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/security_label.sgml b/doc/src/sgml/ref/security_label.sgml index 20a839ff0c..e4eee77932 100644 --- a/doc/src/sgml/ref/security_label.sgml +++ b/doc/src/sgml/ref/security_label.sgml @@ -44,7 +44,7 @@ TABLESPACE <replaceable class="parameter">object_name</replaceable> | TYPE <replaceable class="parameter">object_name</replaceable> | VIEW <replaceable class="parameter">object_name</replaceable> -} IS '<replaceable class="parameter">label</replaceable>' +} IS { '<replaceable class="parameter">label</replaceable>' | NULL } <phrase>where <replaceable>aggregate_signature</replaceable> is:</phrase> @@ -178,11 +178,19 @@ </varlistentry> <varlistentry> - <term><replaceable class="parameter">label</replaceable></term> + <term><literal>'</literal><replaceable class="parameter">label</replaceable><literal>'</literal></term> <listitem> <para> - The new security label, written as a string literal; or <literal>NULL</literal> - to drop the security label. + The new security label, written as a string literal. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>NULL</literal>></term> + <listitem> + <para> + Write <literal>NULL</literal> to drop the security label. </para> </listitem> </varlistentry> -- 2.34.1

view thread (5+ messages)  latest in thread

Message-ID: <87pmauk27g.fsf@wibble.ilmari.org>
Permalink:  ../87pmauk27g.fsf@wibble.ilmari.org/
Also on:    postgresql.org/message-id/87pmauk27g.fsf@wibble.ilmari.org

 ·  · 

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: pgsql-hackers@postgresql.org
  Cc: ilmari@ilmari.org, tgl@sss.pgh.pa.us
  Subject: Re: Clarify deleting comments and security labels in synopsis
  In-Reply-To: <87pmauk27g.fsf@wibble.ilmari.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox