On 2021/04/16 9:15, Bharath Rupireddy wrote:
On Thu, Apr 15, 2021 at 8:19 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
On 2021/04/14 12:54, Bharath Rupireddy wrote:
IMHO, we can push all the TRUNCATE options (ONLY, RESTRICTED, CASCADE,
RESTART/CONTINUE IDENTITY), because it doesn't have any major
challenge(implementation wise) unlike pushing some clauses in
SELECT/UPDATE/DELETE and we already do this on the master. It doesn't
look good and may confuse users, if we push some options and restrict
others. We should have an explicit note in the documentation saying we
push all these options to the remote server. We can leave it to the
user to write TRUNCATE for foreign tables with the appropriate
options. If somebody complains about a problem that they will face
with this behavior, we can revisit.
That's one of the options. But I'm afraid it's hard to drop (revisit)
the feature once it has been released. So if there is no explicit
use case for that, basically I'd like to drop that before release
like we agree to drop unused TRUNCATE_REL_CONTEXT_CASCADING.
Thanks. Looks like the decision is going in the direction of
restricting those options, I will withdraw my point.
We are still discussing whether RESTRICT option should be pushed down to
a foreign data wrapper. But ISTM at least we could reach the consensus about
the drop of extra information for each foreign table. So what about applying
the attached patch and remove the extra information at first?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index bdc4c3620d..7a798530e3 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -2179,24 +2179,19 @@ deparseAnalyzeSql(StringInfo buf, Relation rel, List
**retrieved_attrs)
void
deparseTruncateSql(StringInfo buf,
List *rels,
- List *rels_extra,
DropBehavior behavior,
bool restart_seqs)
{
- ListCell *lc1,
- *lc2;
+ ListCell *cell;
appendStringInfoString(buf, "TRUNCATE ");
- forboth(lc1, rels, lc2, rels_extra)
+ foreach(cell, rels)
{
- Relation rel = lfirst(lc1);
- int extra = lfirst_int(lc2);
+ Relation rel = lfirst(cell);
- if (lc1 != list_head(rels))
+ if (cell != list_head(rels))
appendStringInfoString(buf, ", ");
- if (extra & TRUNCATE_REL_CONTEXT_ONLY)
- appendStringInfoString(buf, "ONLY ");
deparseRelation(buf, rel);
}
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out
b/contrib/postgres_fdw/expected/postgres_fdw.out
index 5070d93239..1a3f5cb4ad 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -8218,13 +8218,13 @@ drop table loc3;
-- test for TRUNCATE
-- ===================================================================
CREATE TABLE tru_rtable0 (id int primary key);
-CREATE TABLE tru_rtable1 (id int primary key);
CREATE FOREIGN TABLE tru_ftable (id int)
SERVER loopback OPTIONS (table_name 'tru_rtable0');
INSERT INTO tru_rtable0 (SELECT x FROM generate_series(1,10) x);
CREATE TABLE tru_ptable (id int) PARTITION BY HASH(id);
CREATE TABLE tru_ptable__p0 PARTITION OF tru_ptable
FOR VALUES WITH (MODULUS 2, REMAINDER 0);
+CREATE TABLE tru_rtable1 (id int primary key);
CREATE FOREIGN TABLE tru_ftable__p1 PARTITION OF tru_ptable
FOR VALUES WITH (MODULUS 2, REMAINDER 1)
SERVER loopback OPTIONS (table_name 'tru_rtable1');
@@ -8388,22 +8388,22 @@ SELECT sum(id) FROM tru_ftable; -- 95
95
(1 row)
-TRUNCATE ONLY tru_ftable; -- truncate only parent portion
-SELECT sum(id) FROM tru_ftable; -- 60
- sum
------
- 60
+TRUNCATE ONLY tru_ftable; -- truncate both parent and child
+SELECT count(*) FROM tru_ftable; -- 0
+ count
+-------
+ 0
(1 row)
INSERT INTO tru_rtable0 (SELECT x FROM generate_series(21,25) x);
-SELECT sum(id) FROM tru_ftable; -- 175
+SELECT sum(id) FROM tru_ftable; -- 115
sum
-----
- 175
+ 115
(1 row)
TRUNCATE tru_ftable; -- truncate both of parent and child
-SELECT count(*) FROM tru_ftable; -- empty
+SELECT count(*) FROM tru_ftable; -- 0
count
-------
0
diff --git a/contrib/postgres_fdw/postgres_fdw.c
b/contrib/postgres_fdw/postgres_fdw.c
index c590f374c6..c521cba3fc 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -401,7 +401,6 @@ static void postgresExplainForeignModify(ModifyTableState
*mtstate,
static void postgresExplainDirectModify(ForeignScanState *node,
ExplainState *es);
static void postgresExecForeignTruncate(List *rels,
-
List *rels_extra,
DropBehavior behavior,
bool restart_seqs);
static bool postgresAnalyzeForeignTable(Relation relation,
@@ -2881,7 +2880,6 @@ postgresExplainDirectModify(ForeignScanState *node,
ExplainState *es)
*/
static void
postgresExecForeignTruncate(List *rels,
- List *rels_extra,
DropBehavior behavior,
bool restart_seqs)
{
@@ -2964,7 +2962,7 @@ postgresExecForeignTruncate(List *rels,
/* Construct the TRUNCATE command string */
initStringInfo(&sql);
- deparseTruncateSql(&sql, rels, rels_extra, behavior, restart_seqs);
+ deparseTruncateSql(&sql, rels, behavior, restart_seqs);
/* Issue the TRUNCATE command to remote server */
do_sql_command(conn, sql.data);
diff --git a/contrib/postgres_fdw/postgres_fdw.h
b/contrib/postgres_fdw/postgres_fdw.h
index 5d44b75314..9591c0f6c2 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -209,7 +209,6 @@ extern void deparseAnalyzeSql(StringInfo buf, Relation rel,
List
**retrieved_attrs);
extern void deparseTruncateSql(StringInfo buf,
List *rels,
- List *rels_extra,
DropBehavior
behavior,
bool restart_seqs);
extern void deparseStringLiteral(StringInfo buf, const char *val);
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql
b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 74590089bd..97c156a472 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -2355,7 +2355,6 @@ drop table loc3;
-- test for TRUNCATE
-- ===================================================================
CREATE TABLE tru_rtable0 (id int primary key);
-CREATE TABLE tru_rtable1 (id int primary key);
CREATE FOREIGN TABLE tru_ftable (id int)
SERVER loopback OPTIONS (table_name 'tru_rtable0');
INSERT INTO tru_rtable0 (SELECT x FROM generate_series(1,10) x);
@@ -2363,6 +2362,7 @@ INSERT INTO tru_rtable0 (SELECT x FROM
generate_series(1,10) x);
CREATE TABLE tru_ptable (id int) PARTITION BY HASH(id);
CREATE TABLE tru_ptable__p0 PARTITION OF tru_ptable
FOR VALUES WITH (MODULUS 2, REMAINDER 0);
+CREATE TABLE tru_rtable1 (id int primary key);
CREATE FOREIGN TABLE tru_ftable__p1 PARTITION OF tru_ptable
FOR VALUES WITH (MODULUS 2, REMAINDER 1)
SERVER loopback OPTIONS (table_name 'tru_rtable1');
@@ -2439,13 +2439,13 @@ INSERT INTO tru_rtable0 (SELECT x FROM
generate_series(5,9) x);
INSERT INTO tru_rtable0_child (SELECT x FROM generate_series(10,14) x);
SELECT sum(id) FROM tru_ftable; -- 95
-TRUNCATE ONLY tru_ftable; -- truncate only parent portion
-SELECT sum(id) FROM tru_ftable; -- 60
+TRUNCATE ONLY tru_ftable; -- truncate both parent and child
+SELECT count(*) FROM tru_ftable; -- 0
INSERT INTO tru_rtable0 (SELECT x FROM generate_series(21,25) x);
-SELECT sum(id) FROM tru_ftable; -- 175
+SELECT sum(id) FROM tru_ftable; -- 115
TRUNCATE tru_ftable; -- truncate both of parent and child
-SELECT count(*) FROM tru_ftable; -- empty
+SELECT count(*) FROM tru_ftable; -- 0
-- cleanup
DROP FOREIGN TABLE tru_ftable_parent, tru_ftable_child,
tru_pk_ftable,tru_ftable__p1,tru_ftable;
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 98882ddab8..3fe373ef00 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -1071,28 +1071,16 @@ EndDirectModify(ForeignScanState *node);
<para>
<programlisting>
void
-ExecForeignTruncate(List *rels, List *rels_extra,
- DropBehavior behavior, bool restart_seqs);
+ExecForeignTruncate(List *rels,
+ DropBehavior behavior,
+ bool restart_seqs);
</programlisting>
Truncate a set of foreign tables specified in <literal>rels</literal>.
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.
+ a foreign table to truncate.
</para>
<para>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 096a6f2891..3d6e7edbae 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -323,7 +323,6 @@ typedef struct ForeignTruncateInfo
{
Oid serverid;
List *rels;
- List *rels_extra;
} ForeignTruncateInfo;
/*
@@ -1620,7 +1619,6 @@ ExecuteTruncate(TruncateStmt *stmt)
{
List *rels = NIL;
List *relids = NIL;
- List *relids_extra = NIL;
List *relids_logged = NIL;
ListCell *cell;
@@ -1654,9 +1652,6 @@ ExecuteTruncate(TruncateStmt *stmt)
rels = lappend(rels, rel);
relids = lappend_oid(relids, myrelid);
- relids_extra = lappend_int(relids_extra, (recurse ?
-
TRUNCATE_REL_CONTEXT_NORMAL :
-
TRUNCATE_REL_CONTEXT_ONLY));
/* Log this relation only if needed for logical decoding */
if (RelationIsLogicallyLogged(rel))
relids_logged = lappend_oid(relids_logged, myrelid);
@@ -1704,8 +1699,6 @@ ExecuteTruncate(TruncateStmt *stmt)
rels = lappend(rels, rel);
relids = lappend_oid(relids, childrelid);
- relids_extra = lappend_int(relids_extra,
-
TRUNCATE_REL_CONTEXT_CASCADING);
/* Log this relation only if needed for logical
decoding */
if (RelationIsLogicallyLogged(rel))
relids_logged =
lappend_oid(relids_logged, childrelid);
@@ -1718,7 +1711,7 @@ ExecuteTruncate(TruncateStmt *stmt)
errhint("Do not specify the ONLY
keyword, or use TRUNCATE ONLY on the partitions directly.")));
}
- ExecuteTruncateGuts(rels, relids, relids_extra, relids_logged,
+ ExecuteTruncateGuts(rels, relids, relids_logged,
stmt->behavior,
stmt->restart_seqs);
/* And close the rels */
@@ -1739,15 +1732,13 @@ ExecuteTruncate(TruncateStmt *stmt)
*
* explicit_rels is the list of Relations to truncate that the command
* specified. relids is the list of Oids corresponding to explicit_rels.
- * relids_extra is the list of integer values that deliver extra information
- * about relations in explicit_rels. relids_logged is the list of Oids
- * (a subset of relids) that require WAL-logging. This is all a bit redundant,
- * but the existing callers have this information handy in this form.
+ * relids_logged is the list of Oids (a subset of relids) that require
+ * WAL-logging. This is all a bit redundant, but the existing callers have
+ * this information handy in this form.
*/
void
ExecuteTruncateGuts(List *explicit_rels,
List *relids,
- List *relids_extra,
List *relids_logged,
DropBehavior behavior, bool
restart_seqs)
{
@@ -1760,8 +1751,6 @@ ExecuteTruncateGuts(List *explicit_rels,
ResultRelInfo *resultRelInfo;
SubTransactionId mySubid;
ListCell *cell;
- ListCell *lc1,
- *lc2;
Oid *logrelids;
/*
@@ -1799,8 +1788,6 @@ 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);
/* Log this relation only if needed for logical
decoding */
if (RelationIsLogicallyLogged(rel))
relids_logged =
lappend_oid(relids_logged, relid);
@@ -1901,11 +1888,9 @@ ExecuteTruncateGuts(List *explicit_rels,
*/
mySubid = GetCurrentSubTransactionId();
- Assert(list_length(rels) == list_length(relids_extra));
- forboth(lc1, rels, lc2, relids_extra)
+ foreach(cell, rels)
{
- Relation rel = (Relation) lfirst(lc1);
- int extra = lfirst_int(lc2);
+ Relation rel = (Relation) lfirst(cell);
/*
* Save OID of partitioned tables for later; nothing else to do
for
@@ -1952,7 +1937,6 @@ ExecuteTruncateGuts(List *explicit_rels,
{
ft_info->serverid = serverid;
ft_info->rels = NIL;
- ft_info->rels_extra = NIL;
}
/*
@@ -1960,7 +1944,6 @@ ExecuteTruncateGuts(List *explicit_rels,
* foreign table belongs to.
*/
ft_info->rels = lappend(ft_info->rels, rel);
- ft_info->rels_extra = lappend_int(ft_info->rels_extra,
extra);
continue;
}
@@ -2044,7 +2027,6 @@ ExecuteTruncateGuts(List *explicit_rels,
Assert(routine->ExecForeignTruncate != NULL);
routine->ExecForeignTruncate(ft_info->rels,
-
ft_info->rels_extra,
behavior,
restart_seqs);
}
diff --git a/src/backend/replication/logical/worker.c
b/src/backend/replication/logical/worker.c
index fb3ba5c415..4dd869aaba 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1795,7 +1795,6 @@ apply_handle_truncate(StringInfo s)
List *rels = NIL;
List *part_rels = NIL;
List *relids = NIL;
- List *relids_extra = NIL;
List *relids_logged = NIL;
ListCell *lc;
@@ -1825,7 +1824,6 @@ 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);
if (RelationIsLogicallyLogged(rel->localrel))
relids_logged = lappend_oid(relids_logged,
rel->localreloid);
@@ -1864,7 +1862,6 @@ 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);
/* Log this relation only if needed for logical
decoding */
if (RelationIsLogicallyLogged(childrel))
relids_logged =
lappend_oid(relids_logged, childrelid);
@@ -1879,7 +1876,6 @@ apply_handle_truncate(StringInfo s)
*/
ExecuteTruncateGuts(rels,
relids,
- relids_extra,
relids_logged,
DROP_RESTRICT,
restart_seqs);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index b808a07e46..14f4b4882f 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -21,16 +21,6 @@
#include "storage/lock.h"
#include "utils/relcache.h"
-/*
- * 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) */
-
struct AlterTableUtilityContext; /* avoid including tcop/utility.h here
*/
@@ -68,7 +58,6 @@ extern void CheckTableNotInUse(Relation rel, const char
*stmt);
extern void ExecuteTruncate(TruncateStmt *stmt);
extern void ExecuteTruncateGuts(List *explicit_rels,
List *relids,
- List
*relids_extra,
List
*relids_logged,
DropBehavior
behavior,
bool
restart_seqs);
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index 4ebbca6de9..4f17becbb8 100644
--- a/src/include/foreign/fdwapi.h
+++ b/src/include/foreign/fdwapi.h
@@ -161,7 +161,6 @@ typedef List *(*ImportForeignSchema_function)
(ImportForeignSchemaStmt *stmt,
Oid serverOid);
typedef void (*ExecForeignTruncate_function) (List *rels,
-
List *rels_extra,
DropBehavior behavior,
bool restart_seqs);
