From 22041f7b8b90d94903cbaf961f59d1be20bb4138 Mon Sep 17 00:00:00 2001 From: Ted Yu Date: Tue, 1 Mar 2022 16:20:50 -0800 Subject: [PATCH] [#11633] Distinguish temp and foreign tables for batch COPY Summary: In CopyFrom(), currently we regard non-YB table as TEMP table. However, this produces confusing message for FOREIGN table: ``` ts1|pid52548|:30139 2022-03-01 09:16:20.854 UTC [52764] WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. ``` This revision checks the Relation type and uses proper table type in the message. Test Plan: ybd --java-test 'org.yb.pgsql.TestPgRegressContribPostgresFdw#schedule' Reviewers: ena, myang Reviewed By: myang Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D15734 --- .../expected/yb_pg_postgres_fdw.out | 22 +++++++++---------- src/postgres/src/backend/commands/copy.c | 9 ++++++-- .../src/backend/utils/misc/pg_yb_utils.c | 6 +++++ src/postgres/src/include/pg_yb_utils.h | 5 +++++ 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/postgres/contrib/postgres_fdw/expected/yb_pg_postgres_fdw.out b/src/postgres/contrib/postgres_fdw/expected/yb_pg_postgres_fdw.out index 5df27be4c2c2..2f49525ef6ab 100644 --- a/src/postgres/contrib/postgres_fdw/expected/yb_pg_postgres_fdw.out +++ b/src/postgres/contrib/postgres_fdw/expected/yb_pg_postgres_fdw.out @@ -7214,7 +7214,7 @@ select tableoid::regclass, * FROM remp2 order by 1, 2, 3; -- Copying into foreign partitions directly should work as well copy remp1 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. select tableoid::regclass, * FROM remp1 order by 1, 2, 3; tableoid | a | b @@ -7245,7 +7245,7 @@ select pg_sleep(1); -- Test basic functionality copy rem2 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. select * from rem2 order by f1; f1 | f2 @@ -7267,10 +7267,10 @@ select pg_sleep(1); -- check constraint is enforced on the remote side, not locally copy rem2 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. copy rem2 from stdin; -- ERROR -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. ERROR: new row for relation "loc2" violates check constraint "loc2_f1positive" DETAIL: Failing row contains (-1, xyzzy). @@ -7307,7 +7307,7 @@ select pg_sleep(1); (1 row) copy rem2 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. select * from rem2 order by f1; f1 | f2 @@ -7336,7 +7336,7 @@ select pg_sleep(1); -- The new values are concatenated with ' triggered !' copy rem2 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. select * from rem2 order by f1; f1 | f2 @@ -7358,7 +7358,7 @@ select pg_sleep(1); -- Nothing happens copy rem2 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. select * from rem2 order by f1; f1 | f2 @@ -7379,7 +7379,7 @@ select pg_sleep(1); -- The new values are concatenated with ' triggered !' copy rem2 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. select * from rem2 order by f1; f1 | f2 @@ -7401,7 +7401,7 @@ select pg_sleep(2); -- Nothing happens copy rem2 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. select * from rem2 order by f1; f1 | f2 @@ -7427,7 +7427,7 @@ select pg_sleep(1); (1 row) copy rem2 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. select * from rem2 order by f1; f1 | f2 @@ -7448,7 +7448,7 @@ begin; create foreign table rem3 (f1 int, f2 text) server loopback options(table_name 'loc3'); copy rem3 from stdin; -WARNING: Batched COPY is not supported on temporary tables. Defaulting to using one transaction for the entire copy. +WARNING: Batched COPY is not supported on foreign tables. Defaulting to using one transaction for the entire copy. HINT: Either copy onto non-temporary table or set rows_per_transaction option to `0` to disable batching and remove this warning. commit; select * from rem3 order by f1; diff --git a/src/postgres/src/backend/commands/copy.c b/src/postgres/src/backend/commands/copy.c index 24c636a80020..ed6c98b488c3 100644 --- a/src/postgres/src/backend/commands/copy.c +++ b/src/postgres/src/backend/commands/copy.c @@ -2646,12 +2646,17 @@ CopyFrom(CopyState cstate) int batch_size = 0; if (!IsYBRelation(resultRelInfo->ri_RelationDesc)) + { + Assert(resultRelInfo->ri_RelationDesc->rd_rel->relpersistence == RELPERSISTENCE_TEMP || + resultRelInfo->ri_RelationDesc->rd_rel->relkind == RELKIND_FOREIGN_TABLE); ereport(WARNING, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("Batched COPY is not supported on temporary tables. " - "Defaulting to using one transaction for the entire copy."), + errmsg("Batched COPY is not supported on %s tables. " + "Defaulting to using one transaction for the entire copy.", + YbIsTempRelation(resultRelInfo->ri_RelationDesc) ? "temporary" : "foreign"), errhint("Either copy onto non-temporary table or set rows_per_transaction " "option to `0` to disable batching and remove this warning."))); + } else if (YBIsDataSent()) ereport(WARNING, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), diff --git a/src/postgres/src/backend/utils/misc/pg_yb_utils.c b/src/postgres/src/backend/utils/misc/pg_yb_utils.c index 16155e111392..f3c5d897b29e 100644 --- a/src/postgres/src/backend/utils/misc/pg_yb_utils.c +++ b/src/postgres/src/backend/utils/misc/pg_yb_utils.c @@ -150,6 +150,12 @@ IsYBBackedRelation(Relation relation) relation->rd_rel->relpersistence != RELPERSISTENCE_TEMP); } +bool +YbIsTempRelation(Relation relation) +{ + return relation->rd_rel->relpersistence == RELPERSISTENCE_TEMP; +} + bool IsRealYBColumn(Relation rel, int attrNum) { return (attrNum > 0 && !TupleDescAttr(rel->rd_att, attrNum - 1)->attisdropped) || diff --git a/src/postgres/src/include/pg_yb_utils.h b/src/postgres/src/include/pg_yb_utils.h index 3d5b34de7c54..10fc8f714ba0 100644 --- a/src/postgres/src/include/pg_yb_utils.h +++ b/src/postgres/src/include/pg_yb_utils.h @@ -141,6 +141,11 @@ extern bool IsYBRelation(Relation relation); */ extern bool IsYBBackedRelation(Relation relation); +/* + * Returns whether a relation is TEMP table + */ +extern bool YbIsTempRelation(Relation relation); + /* * Returns whether a relation's attribute is a real column in the backing * YugaByte table. (It implies we can both read from and write to it).