Re: TRUNCATE on foreign table

On Thu, Apr 22, 2021 at 4:39 AM Bharath Rupireddy <
bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:

> On Thu, Apr 22, 2021 at 2:26 PM Justin Pryzby <pryzby(at)telsasoft(dot)com>
> wrote:
> >
> > On Thu, Apr 22, 2021 at 03:36:25PM +0900, Fujii Masao wrote:
> > > diff --git a/doc/src/sgml/fdwhandler.sgml
> b/doc/src/sgml/fdwhandler.sgml
> > > index 553524553b..69aa66e73e 100644
> > > --- a/doc/src/sgml/fdwhandler.sgml
> > > +++ b/doc/src/sgml/fdwhandler.sgml
> > > @@ -1076,27 +1076,25 @@ ExecForeignTruncate(List *rels,
> > > bool restart_seqs);
> > > <para>
> > > - <literal>behavior</literal> defines how foreign tables should
> > > - be truncated, using as possible values
> <literal>DROP_RESTRICT</literal>,
> > > - which means that <literal>RESTRICT</literal> option is specified,
> > > - and <literal>DROP_CASCADE</literal>, which means that
> > > - <literal>CASCADE</literal> option is specified, in
> > > - <command>TRUNCATE</command> command.
> > > + <literal>behavior</literal> is either
> <literal>DROP_RESTRICT</literal>
> > > + or <literal>DROP_CASCADE</literal>, which indicates that the
> > > + <literal>RESTRICT</literal> or <literal>CASCADE</literal> option
> was
> > > + requested in the original <command>TRUNCATE</command> command,
> > > + respectively.
> >
> > Now that I reread this, I would change "which indicates" to "indicating".
>
> +1.
>
> > > - <literal>restart_seqs</literal> is set to <literal>true</literal>
> > > - if <literal>RESTART IDENTITY</literal> option is specified in
> > > - <command>TRUNCATE</command> command. It is
> <literal>false</literal>
> > > - if <literal>CONTINUE IDENTITY</literal> option is specified.
> > > + If <literal>restart_seqs</literal> is <literal>true</literal>,
> > > + the original <command>TRUNCATE</command> command requested the
> > > + <literal>RESTART IDENTITY</literal> option, otherwise
> > > + <literal>CONTINUE IDENTITY</literal> option.
> >
> > should it say "specified" instead of requested ?
> > Or should it say "requested the RESTART IDENTITY behavior" ?
> >
> > Also, I think it should say "..otherwise, the CONTINUE IDENTITY behavior
> was
> > requested".
>
> The original TRUNCATE document uses this - "When RESTART IDENTITY is
> specified"
>
> IMO the following looks better: "If restart_seqs is true, RESTART
> IDENTITY was specified in the original TRUNCATE command, otherwise
> CONTINUE IDENTITY was specified."
>
> > > +++ b/doc/src/sgml/ref/truncate.sgml
> > > @@ -173,7 +173,7 @@ TRUNCATE [ TABLE ] [ ONLY ] <replaceable
> class="parameter">name</replaceable> [
> > >
> > > <para>
> > > <command>TRUNCATE</command> can be used for foreign tables if
> > > - the foreign data wrapper supports, for instance,
> > > + supported by the foreign data wrapper, for instance,
> > > see <xref linkend="postgres-fdw"/>.
> >
> > what does "for instance" mean here? I think it should be removed.
>
> +1.
>
> > > +++ b/doc/src/sgml/fdwhandler.sgml
> > > @@ -1111,6 +1099,15 @@ ExecForeignTruncate(List *rels, List
> *rels_extra,
> > > if <literal>CONTINUE IDENTITY</literal> option is specified.
> > > </para>
> > >
> > > + <para>
> > > + Note that information about <literal>ONLY</literal> options
> specified
> > > + in the original <command>TRUNCATE</command> command is not
> passed to
> > > + <function>ExecForeignTruncate</function>. This is the same
> behavior as
> > > + for the callback functions for <command>SELECT</command>,
> > > + <command>UPDATE</command> and <command>DELETE</command> on
> >
> > There's an extra space before DELETE
>
> Good catch! Extra space after "and" and before "<command>".
>
> > > diff --git a/doc/src/sgml/postgres-fdw.sgml
> b/doc/src/sgml/postgres-fdw.sgml
> > > index 5320accf6f..d03731b7d4 100644
> > > --- a/doc/src/sgml/postgres-fdw.sgml
> > > +++ b/doc/src/sgml/postgres-fdw.sgml
> > > @@ -69,6 +69,13 @@
> > > have privileges to do these things.)
> > > </para>
> > >
> > > + <para>
> > > + Note that <literal>ONLY</literal> option specified in
> >
> > add "the" to say: "the ONLY"
>
> +1.
>

Since 'the only option' is legitimate English phrase, I think the following
would be clearer:

Note that the option <literal>ONLY</literal> ...

Cheers

>
> > > + <command>SELECT</command>, <command>UPDATE</command>,
> > > + <command>DELETE</command> or <command>TRUNCATE</command>
> > > + has no effect when accessing or modifyung the remote table.
> >
> > modifying
>
> Good catch!
>
> With Regards,
> Bharath Rupireddy.
> EnterpriseDB: http://www.enterprisedb.com
>