Skip to content

Commit

Permalink
[#11633] Distinguish temp and foreign tables for batch COPY
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@tedyu
tedyu committed Mar 3, 2022
1 parent d68c7dd commit 22041f7
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 13 deletions.
22 changes: 11 additions & 11 deletions src/postgres/contrib/postgres_fdw/expected/yb_pg_postgres_fdw.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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).
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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;
Expand Down
9 changes: 7 additions & 2 deletions src/postgres/src/backend/commands/copy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 6 additions & 0 deletions src/postgres/src/backend/utils/misc/pg_yb_utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) ||
Expand Down
5 changes: 5 additions & 0 deletions src/postgres/src/include/pg_yb_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down

0 comments on commit 22041f7

Please sign in to comment.