On Sun, Apr 11, 2021 at 03:45:36PM +0530, Bharath Rupireddy wrote:
> On Sun, Apr 11, 2021 at 9:47 AM Justin Pryzby <pry...@telsasoft.com> wrote:
> > Also, you currently test:
> > > + if (extra & TRUNCATE_REL_CONTEXT_ONLY)
> >
> > but TRUNCATE_REL_ aren't indepedent bits, so shouldn't be tested with "&".
>
> Yeah this is an issue. We could just change the #defines to values
> 0x0001, 0x0002, 0x0004, 0x0008 ... 0x0020 and so on and then testing
> with & would work. So, this way, more than option can be multiplexed
> into the same int value. To multiplex, we need to think: will there be
> a scenario where a single rel in the truncate can have multiple
> options at a time and do we want to distinguish these options while
> deparsing?
>
> #define TRUNCATE_REL_CONTEXT_NORMAL 0x0001 /* specified without
> ONLY clause */
> #define TRUNCATE_REL_CONTEXT_ONLY 0x0002 /* specified with
> ONLY clause */
> #define TRUNCATE_REL_CONTEXT_CASCADING 0x0004 /* not specified
> but truncated
>
> And I'm not sure what's the agreement on retaining or removing #define
> values, currently I see only TRUNCATE_REL_CONTEXT_ONLY is being used,
> others are just being set but not used. As I said upthread, it will be
> good to remove the unused macros/enums, retain only the ones that are
> used, especially TRUNCATE_REL_CONTEXT_CASCADING this is not required I
> feel, because we can add the child partitions that are foreign tables
> to relids as just normal foreign tables with TRUNCATE_REL_CONTEXT_ONLY
> option.
Converting to "bits" would collapse TRUNCATE_REL_CONTEXT_ONLY and
TRUNCATE_REL_CONTEXT_NORMAL into a single bit. TRUNCATE_REL_CONTEXT_CASCADING
could optionally be removed.
+1 to convert to bits instead of changing "&" to "==".
--
Justin
>From 26e1015de344352c4cd73deda28ad59594914067 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 12 Apr 2021 19:07:34 -0500
Subject: [PATCH 1/2] Convert TRUNCATE_REL_CONTEXT_* to bits
The values were defined as consecutive integers, but TRUNCATE_REL_CONTEXT_ONLY
was tested with bitwise test. If specified as bits, NORMAL vs ONLY is
redundant (there's only one bit). And CASCADING was unused.
See also: 8ff1c94649f5c9184ac5f07981d8aea9dfd7ac19
---
src/backend/commands/tablecmds.c | 9 +++------
src/backend/replication/logical/worker.c | 4 ++--
src/include/commands/tablecmds.h | 6 +-----
3 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 096a6f2891..e8215d8dc8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1654,8 +1654,7 @@ ExecuteTruncate(TruncateStmt *stmt)
rels = lappend(rels, rel);
relids = lappend_oid(relids, myrelid);
- relids_extra = lappend_int(relids_extra, (recurse ?
- TRUNCATE_REL_CONTEXT_NORMAL :
+ relids_extra = lappend_int(relids_extra, (recurse ? 0 :
TRUNCATE_REL_CONTEXT_ONLY));
/* Log this relation only if needed for logical decoding */
if (RelationIsLogicallyLogged(rel))
@@ -1704,8 +1703,7 @@ ExecuteTruncate(TruncateStmt *stmt)
rels = lappend(rels, rel);
relids = lappend_oid(relids, childrelid);
- relids_extra = lappend_int(relids_extra,
- TRUNCATE_REL_CONTEXT_CASCADING);
+ relids_extra = lappend_int(relids_extra, 0);
/* Log this relation only if needed for logical decoding */
if (RelationIsLogicallyLogged(rel))
relids_logged = lappend_oid(relids_logged, childrelid);
@@ -1799,8 +1797,7 @@ ExecuteTruncateGuts(List *explicit_rels,
truncate_check_activity(rel);
rels = lappend(rels, rel);
relids = lappend_oid(relids, relid);
- relids_extra = lappend_int(relids_extra,
- TRUNCATE_REL_CONTEXT_CASCADING);
+ relids_extra = lappend_int(relids_extra, 0);
/* Log this relation only if needed for logical decoding */
if (RelationIsLogicallyLogged(rel))
relids_logged = lappend_oid(relids_logged, relid);
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index fb3ba5c415..986b634525 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1825,7 +1825,7 @@ apply_handle_truncate(StringInfo s)
remote_rels = lappend(remote_rels, rel);
rels = lappend(rels, rel->localrel);
relids = lappend_oid(relids, rel->localreloid);
- relids_extra = lappend_int(relids_extra, TRUNCATE_REL_CONTEXT_NORMAL);
+ relids_extra = lappend_int(relids_extra, 0);
if (RelationIsLogicallyLogged(rel->localrel))
relids_logged = lappend_oid(relids_logged, rel->localreloid);
@@ -1864,7 +1864,7 @@ apply_handle_truncate(StringInfo s)
rels = lappend(rels, childrel);
part_rels = lappend(part_rels, childrel);
relids = lappend_oid(relids, childrelid);
- relids_extra = lappend_int(relids_extra, TRUNCATE_REL_CONTEXT_CASCADING);
+ relids_extra = lappend_int(relids_extra, 0);
/* Log this relation only if needed for logical decoding */
if (RelationIsLogicallyLogged(childrel))
relids_logged = lappend_oid(relids_logged, childrelid);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index b808a07e46..9aa9f3c6c7 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -25,11 +25,7 @@
* These values indicate how a relation was specified as the target to
* truncate in TRUNCATE command.
*/
-#define TRUNCATE_REL_CONTEXT_NORMAL 1 /* specified without ONLY clause */
-#define TRUNCATE_REL_CONTEXT_ONLY 2 /* specified with ONLY clause */
-#define TRUNCATE_REL_CONTEXT_CASCADING 3 /* not specified but truncated
- * due to dependency (e.g.,
- * partition table) */
+#define TRUNCATE_REL_CONTEXT_ONLY 0x01 /* specified with ONLY clause */
struct AlterTableUtilityContext; /* avoid including tcop/utility.h here */
--
2.17.0
>From 8f332a4c9a8690ade17421528b1d375ddd992cce Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Thu, 8 Apr 2021 10:10:58 -0500
Subject: [PATCH 2/2] WIP doc review: Allow TRUNCATE command to truncate
foreign tables.
8ff1c94649f5c9184ac5f07981d8aea9dfd7ac19
---
doc/src/sgml/fdwhandler.sgml | 55 ++++++++++++++--------------------
doc/src/sgml/postgres-fdw.sgml | 2 +-
doc/src/sgml/ref/truncate.sgml | 2 +-
3 files changed, 25 insertions(+), 34 deletions(-)
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 98882ddab8..69feefe66b 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -1075,46 +1075,37 @@ ExecForeignTruncate(List *rels, List *rels_extra,
DropBehavior behavior, bool restart_seqs);
</programlisting>
- Truncate a set of foreign tables specified in <literal>rels</literal>.
+ Truncate foreign tables.
This function is called when <xref linkend="sql-truncate"/> is executed
- on foreign tables. <literal>rels</literal> is the list of
- <structname>Relation</structname> data structure that indicates
- a foreign table to truncate. <literal>rels_extra</literal> the list of
- <literal>int</literal> value, which delivers extra information about
- a foreign table to truncate. Possible values are
- <literal>TRUNCATE_REL_CONTEXT_NORMAL</literal>, which means that
- the foreign table is specified WITHOUT <literal>ONLY</literal> clause
- in <command>TRUNCATE</command>,
- <literal>TRUNCATE_REL_CONTEXT_ONLY</literal>, which means that
- the foreign table is specified WITH <literal>ONLY</literal> clause,
- and <literal>TRUNCATE_REL_CONTEXT_CASCADING</literal>,
- which means that the foreign table is not specified explicitly,
- but will be truncated due to dependency (for example, partition table).
- There is one-to-one mapping between <literal>rels</literal> and
- <literal>rels_extra</literal>. The number of entries is the same
- between the two lists.
- </para>
-
- <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
+ on a foreign table. <literal>rels</literal> is a list of
+ <structname>Relation</structname> data structures for each
+ foreign tables to be truncated.
+ <literal>rels_extra</literal> is a list of <literal>int</literal> values
+ of the same length as <literal>rels</literal>. Each element of
+ <literal>rels_extra</literal> is a bitmask of flags and provides extra
+ information about the corresponding foreign table. Currently, the only
+ defined flag is <literal>TRUNCATE_REL_CONTEXT_ONLY</literal>, which means
+ that the foreign table was specified with the <literal>ONLY</literal>
+ clause in the original <command>TRUNCATE</command> command.
+ </para>
+
+ <para>
+ <literal>behavior</literal> is either
+ <literal>DROP_RESTRICT</literal> or <literal>DROP_CASCADE</literal>.
+ <literal>DROP_CASCADE</literal> indicates that the
+ <literal>CASCADE</literal> option was specified in the original
<command>TRUNCATE</command> command.
</para>
<para>
- <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 included the
+ <literal>RESTART IDENTITY</literal> option.
</para>
<para>
- <command>TRUNCATE</command> invokes
- <function>ExecForeignTruncate</function> once per foreign server
- that foreign tables to truncate belong to. This means that all foreign
+ <function>ExecForeignTruncate</function> is invoked once per foreign server
+ for which foreign tables are to be truncated. This means that all foreign
tables included in <literal>rels</literal> must belong to the same
server.
</para>
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5320accf6f..116434f658 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -452,7 +452,7 @@ OPTIONS (ADD password_required 'false');
<listitem>
<para>
This option controls whether <filename>postgres_fdw</filename> allows
- foreign tables to be truncated using <command>TRUNCATE</command>
+ foreign tables to be truncated using the <command>TRUNCATE</command>
command. It can be specified for a foreign table or a foreign server.
A table-level option overrides a server-level option.
The default is <literal>true</literal>.
diff --git a/doc/src/sgml/ref/truncate.sgml b/doc/src/sgml/ref/truncate.sgml
index acf3633be4..9d846f88c9 100644
--- a/doc/src/sgml/ref/truncate.sgml
+++ 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"/>.
</para>
</refsect1>
--
2.17.0
