pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedClarify deleting comments and security labels in synopsis
5+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2023-01-31 856ca51a23 e4035a9ca4 26da294098 5fb1809eb9 dbe2946443 c1827e9cd2 Doc: clarify use of NULL to drop comments and security labels.
* Clarify deleting comments and security labels in synopsis
@ 2023-01-31 17:07 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 1 reply; 5+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2023-01-31 17:07 UTC (permalink / raw)
To: pgsql-hackers
Hi Hackers,
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.
- ilmari
Attachments:
0001-Clarify-that-COMMENT-and-SECURITY-LABEL-can-be-set-t.patch (text/x-diff, 1.9K) download
From cb02bd9aae85a48355dede9dd13db809f6ada35b 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] 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).
---
doc/src/sgml/ref/comment.sgml | 2 +-
doc/src/sgml/ref/security_label.sgml | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml
index 7499da1d62..f5daea7a7e 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>
diff --git a/doc/src/sgml/ref/security_label.sgml b/doc/src/sgml/ref/security_label.sgml
index 20a839ff0c..3df5ed2e9c 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>
--
2.34.1
* Re: Clarify deleting comments and security labels in synopsis
@ 2023-01-31 17:35 Tom Lane <tgl@sss.pgh.pa.us>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 1 reply; 5+ messages in thread
From: Tom Lane @ 2023-01-31 17:35 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers
=?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.
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Clarify deleting comments and security labels in synopsis
@ 2023-01-31 18:07 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 5+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2023-01-31 18:07 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
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
* Re: Clarify deleting comments and security labels in synopsis
@ 2023-01-31 18:52 Tom Lane <tgl@sss.pgh.pa.us>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
0 siblings, 1 reply; 5+ messages in thread
From: Tom Lane @ 2023-01-31 18:52 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>; +Cc: pgsql-hackers
=?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> 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:
> Makes sense. Something like the attached v2?
WFM, will push.
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Clarify deleting comments and security labels in synopsis
@ 2023-02-01 10:50 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 5+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2023-02-01 10:50 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
Tom Lane <tgl@sss.pgh.pa.us> writes:
> =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org> writes:
>> Tom Lane <tgl@sss.pgh.pa.us> writes:
>>> 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:
>
>> Makes sense. Something like the attached v2?
>
> WFM, will push.
Thanks!
> regards, tom lane
- ilmari
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2023-02-01 10:50 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-01-31 17:07 Clarify deleting comments and security labels in synopsis Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2023-01-31 17:35 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-01-31 18:07 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2023-01-31 18:52 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-02-01 10:50 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
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