On 2017/08/30 17:20, Etsuro Fujita wrote:
Here is a patch to skip the CheckValidResultRel checks for a
target table if it's a foreign partition to perform tuple-routing for,
as proposed by Robert.
In the patch, to skip the checks, I passed to CheckValidResultRel a new
flag indicating whether the target relation is a partition to do
tuple-routing for, but while updating another patch for tuple-routing
for foreign partitions in PG11, I noticed that it would be better to
pass ResultRelInfo than that flag. The reason is: (1) we can see
whether the relation is such a partition by looking at the
ResultRelInfo's ri_PartitionRoot, instead of that flag, and (2) this is
for tuple-routing for foreign partitions, but we could extend that
function with that argument to do the checks without throwing an error
and save the result in a new member of ResultRelInfo (eg,
ri_PartitionIsValid) so that we can use that info to determine in
ExecInsert whether a foreign partition chosen by ExecFindPartition is
valid for tuple-routing. So, I updated the patch that way. Attached is
an updated version of the patch.
(I discussed about lazy initialization of partition ResultRelInfos
before, but I changed my mind because I noticed that the change to
EXPLAIN for INSERT into a partitioned table, which I'd like to propose
in the tuple-routing-for-foereign-partitions patch, needs partition
ResultRelInfos.)
Best regards,
Etsuro Fujita
*** /dev/null
--- b/contrib/file_fdw/data/list1.csv
***************
*** 0 ****
--- 1,2 ----
+ 1,foo
+ 1,bar
*** /dev/null
--- b/contrib/file_fdw/data/list2.bad
***************
*** 0 ****
--- 1,2 ----
+ 2,baz
+ 1,qux
*** /dev/null
--- b/contrib/file_fdw/data/list2.csv
***************
*** 0 ****
--- 1,2 ----
+ 2,baz
+ 2,qux
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
***************
*** 162,167 **** SELECT tableoid::regclass, * FROM agg FOR UPDATE;
--- 162,188 ----
ALTER FOREIGN TABLE agg_csv NO INHERIT agg;
DROP TABLE agg;
+ -- declarative partitioning tests
+ SET ROLE regress_file_fdw_superuser;
+ CREATE TABLE pt (a int, b text) partition by list (a);
+ CREATE FOREIGN TABLE p1 partition of pt for values in (1) SERVER file_server
+ OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',');
+ CREATE TABLE p2 partition of pt for values in (2);
+ SELECT tableoid::regclass, * FROM pt;
+ SELECT tableoid::regclass, * FROM p1;
+ SELECT tableoid::regclass, * FROM p2;
+ COPY pt FROM '@abs_srcdir@/data/list2.bad' with (format 'csv', delimiter
','); -- ERROR
+ COPY pt FROM '@abs_srcdir@/data/list2.csv' with (format 'csv', delimiter ',');
+ SELECT tableoid::regclass, * FROM pt;
+ SELECT tableoid::regclass, * FROM p1;
+ SELECT tableoid::regclass, * FROM p2;
+ INSERT INTO pt VALUES (1, 'xyzzy'); -- ERROR
+ INSERT INTO pt VALUES (2, 'xyzzy');
+ SELECT tableoid::regclass, * FROM pt;
+ SELECT tableoid::regclass, * FROM p1;
+ SELECT tableoid::regclass, * FROM p2;
+ DROP TABLE pt;
+
-- privilege tests
SET ROLE regress_file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
***************
*** 289,294 **** SELECT tableoid::regclass, * FROM agg FOR UPDATE;
--- 289,375 ----
ALTER FOREIGN TABLE agg_csv NO INHERIT agg;
DROP TABLE agg;
+ -- declarative partitioning tests
+ SET ROLE regress_file_fdw_superuser;
+ CREATE TABLE pt (a int, b text) partition by list (a);
+ CREATE FOREIGN TABLE p1 partition of pt for values in (1) SERVER file_server
+ OPTIONS (format 'csv', filename '@abs_srcdir@/data/list1.csv', delimiter ',');
+ CREATE TABLE p2 partition of pt for values in (2);
+ SELECT tableoid::regclass, * FROM pt;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ (2 rows)
+
+ SELECT tableoid::regclass, * FROM p1;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ (2 rows)
+
+ SELECT tableoid::regclass, * FROM p2;
+ tableoid | a | b
+ ----------+---+---
+ (0 rows)
+
+ COPY pt FROM '@abs_srcdir@/data/list2.bad' with (format 'csv', delimiter
','); -- ERROR
+ ERROR: cannot route inserted tuples to a foreign table
+ CONTEXT: COPY pt, line 2: "1,qux"
+ COPY pt FROM '@abs_srcdir@/data/list2.csv' with (format 'csv', delimiter ',');
+ SELECT tableoid::regclass, * FROM pt;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ p2 | 2 | baz
+ p2 | 2 | qux
+ (4 rows)
+
+ SELECT tableoid::regclass, * FROM p1;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ (2 rows)
+
+ SELECT tableoid::regclass, * FROM p2;
+ tableoid | a | b
+ ----------+---+-----
+ p2 | 2 | baz
+ p2 | 2 | qux
+ (2 rows)
+
+ INSERT INTO pt VALUES (1, 'xyzzy'); -- ERROR
+ ERROR: cannot route inserted tuples to a foreign table
+ INSERT INTO pt VALUES (2, 'xyzzy');
+ SELECT tableoid::regclass, * FROM pt;
+ tableoid | a | b
+ ----------+---+-------
+ p1 | 1 | foo
+ p1 | 1 | bar
+ p2 | 2 | baz
+ p2 | 2 | qux
+ p2 | 2 | xyzzy
+ (5 rows)
+
+ SELECT tableoid::regclass, * FROM p1;
+ tableoid | a | b
+ ----------+---+-----
+ p1 | 1 | foo
+ p1 | 1 | bar
+ (2 rows)
+
+ SELECT tableoid::regclass, * FROM p2;
+ tableoid | a | b
+ ----------+---+-------
+ p2 | 2 | baz
+ p2 | 2 | qux
+ p2 | 2 | xyzzy
+ (3 rows)
+
+ DROP TABLE pt;
-- privilege tests
SET ROLE regress_file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***************
*** 1097,1104 **** InitPlan(QueryDesc *queryDesc, int eflags)
* CheckValidRowMarkRel.
*/
void
! CheckValidResultRel(Relation resultRel, CmdType operation)
{
TriggerDesc *trigDesc = resultRel->trigdesc;
FdwRoutine *fdwroutine;
--- 1097,1105 ----
* CheckValidRowMarkRel.
*/
void
! CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation)
{
+ Relation resultRel = resultRelInfo->ri_RelationDesc;
TriggerDesc *trigDesc = resultRel->trigdesc;
FdwRoutine *fdwroutine;
***************
*** 1169,1178 **** CheckValidResultRel(Relation resultRel, CmdType operation)
break;
case RELKIND_FOREIGN_TABLE:
/* Okay only if the FDW supports it */
! fdwroutine = GetFdwRoutineForRelation(resultRel, false);
switch (operation)
{
case CMD_INSERT:
if (fdwroutine->ExecForeignInsert ==
NULL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
--- 1170,1185 ----
break;
case RELKIND_FOREIGN_TABLE:
/* Okay only if the FDW supports it */
! fdwroutine = resultRelInfo->ri_FdwRoutine;
switch (operation)
{
case CMD_INSERT:
+ /*
+ * If foreign partition to do
tuple-routing for, skip the
+ * check; it's disallowed elsewhere.
+ */
+ if (resultRelInfo->ri_PartitionRoot)
+ break;
if (fdwroutine->ExecForeignInsert ==
NULL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
***************
*** 3308,3318 **** ExecSetupPartitionTupleRouting(Relation rel,
part_tupdesc = RelationGetDescr(partrel);
/*
- * Verify result relation is a valid target for the current
operation.
- */
- CheckValidResultRel(partrel, CMD_INSERT);
-
- /*
* Save a tuple conversion map to convert a tuple routed to this
* partition from the parent's type to the partition's.
*/
--- 3315,3320 ----
***************
*** 3325,3332 **** ExecSetupPartitionTupleRouting(Relation rel,
rel,
estate->es_instrument);
! estate->es_leaf_result_relations =
! lappend(estate->es_leaf_result_relations,
leaf_part_rri);
/*
* Open partition indices (remember we do not support ON
CONFLICT in
--- 3327,3336 ----
rel,
estate->es_instrument);
! /*
! * Verify result relation is a valid target for INSERT.
! */
! CheckValidResultRel(leaf_part_rri, CMD_INSERT);
/*
* Open partition indices (remember we do not support ON
CONFLICT in
***************
*** 3337,3342 **** ExecSetupPartitionTupleRouting(Relation rel,
--- 3341,3349 ----
leaf_part_rri->ri_IndexRelationDescs == NULL)
ExecOpenIndices(leaf_part_rri, false);
+ estate->es_leaf_result_relations =
+ lappend(estate->es_leaf_result_relations,
leaf_part_rri);
+
leaf_part_rri++;
i++;
}
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
***************
*** 1854,1860 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int
eflags)
/*
* Verify result relation is a valid target for the current
operation
*/
! CheckValidResultRel(resultRelInfo->ri_RelationDesc, operation);
/*
* If there are indices on the result relation, open them and
save
--- 1854,1860 ----
/*
* Verify result relation is a valid target for the current
operation
*/
! CheckValidResultRel(resultRelInfo, operation);
/*
* If there are indices on the result relation, open them and
save
*** a/src/include/executor/executor.h
--- b/src/include/executor/executor.h
***************
*** 177,183 **** extern void ExecutorEnd(QueryDesc *queryDesc);
extern void standard_ExecutorEnd(QueryDesc *queryDesc);
extern void ExecutorRewind(QueryDesc *queryDesc);
extern bool ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation);
! extern void CheckValidResultRel(Relation resultRel, CmdType operation);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
--- 177,183 ----
extern void standard_ExecutorEnd(QueryDesc *queryDesc);
extern void ExecutorRewind(QueryDesc *queryDesc);
extern bool ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation);
! extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType
operation);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
