Skip to content

Commit a4db7fe

Browse files
alvherrealvherre
committed
Disallow COPY FREEZE on partitioned tables
This didn't actually work: COPY would fail to flush the right files, and instead would try to flush a non-existing file, causing the whole transaction to fail. Cope by raising an error as soon as the command is sent instead, to avoid a nasty later surprise. Of course, it would be much better to make it work, but we don't have a patch for that yet, and we don't know if we'll want to backpatch one when we do. Reported-by: Tomas Vondra Author: David Rowley Reviewed-by: Amit Langote, Steve Singer, Tomas Vondra
1 parent 6534d54 commit a4db7fe

File tree

5 files changed

+84
-5
lines changed

5 files changed

+84
-5
lines changed

‎doc/src/sgml/perform.sgml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1535,8 +1535,8 @@ SELECT * FROM x, y, a, b, c WHERE something AND somethingelse;
15351535
needs to be written, because in case of an error, the files
15361536
containing the newly loaded data will be removed anyway.
15371537
However, this consideration only applies when
1538-
<xref linkend="guc-wal-level"/> is <literal>minimal</literal> as all commands
1539-
must write WAL otherwise.
1538+
<xref linkend="guc-wal-level"/> is <literal>minimal</literal> for
1539+
non-partitioned tables as all commands must write WAL otherwise.
15401540
</para>
15411541

15421542
</sect2>

‎doc/src/sgml/ref/copy.sgml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,9 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
224224
This is intended as a performance option for initial data loading.
225225
Rows will be frozen only if the table being loaded has been created
226226
or truncated in the current subtransaction, there are no cursors
227-
open and there are no older snapshots held by this transaction.
227+
open and there are no older snapshots held by this transaction. It is
228+
currently not possible to perform a <command>COPY FREEZE</command> on
229+
a partitioned table.
228230
</para>
229231
<para>
230232
Note that all other sessions will immediately be able to see the data

‎src/backend/commands/copy.c

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2397,11 +2397,20 @@ CopyFrom(CopyState cstate)
23972397
* go into pages containing tuples from any other transactions --- but this
23982398
* must be the case if we have a new table or new relfilenode, so we need
23992399
* no additional work to enforce that.
2400+
*
2401+
* We currently don't support this optimization if the COPY target is a
2402+
* partitioned table as we currently only lazily initialize partition
2403+
* information when routing the first tuple to the partition. We cannot
2404+
* know at this stage if we can perform this optimization. It should be
2405+
* possible to improve on this, but it does mean maintaining heap insert
2406+
* option flags per partition and setting them when we first open the
2407+
* partition.
24002408
*----------
24012409
*/
24022410
/* createSubid is creation check, newRelfilenodeSubid is truncation check */
2403-
if (cstate->rel->rd_createSubid != InvalidSubTransactionId ||
2404-
cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId)
2411+
if (cstate->rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE &&
2412+
(cstate->rel->rd_createSubid != InvalidSubTransactionId ||
2413+
cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId))
24052414
{
24062415
hi_options |= HEAP_INSERT_SKIP_FSM;
24072416
if (!XLogIsNeeded())
@@ -2421,6 +2430,22 @@ CopyFrom(CopyState cstate)
24212430
*/
24222431
if (cstate->freeze)
24232432
{
2433+
/*
2434+
* We currently disallow COPY FREEZE on partitioned tables. The
2435+
* reason for this is that we've simply not yet opened the partitions
2436+
* to determine if the optimization can be applied to them. We could
2437+
* go and open them all here, but doing so may be quite a costly
2438+
* overhead for small copies. In any case, we may just end up routing
2439+
* tuples to a small number of partitions. It seems better just to
2440+
* raise an ERROR for partitioned tables.
2441+
*/
2442+
if (cstate->rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
2443+
{
2444+
ereport(ERROR,
2445+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
2446+
errmsg("cannot perform FREEZE on a partitioned table")));
2447+
}
2448+
24242449
/*
24252450
* Tolerate one registration for the benefit of FirstXactSnapshot.
24262451
* Scan-bearing queries generally create at least two registrations,

‎src/test/regress/input/copy.source

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,3 +133,32 @@ this is just a line full of junk that would error out if parsed
133133
\.
134134

135135
copy copytest3 to stdout csv header;
136+
137+
-- test copy from with a partitioned table
138+
create table parted_copytest (
139+
a int,
140+
b int,
141+
c text
142+
) partition by list (b);
143+
144+
create table parted_copytest_a1 (c text, b int, a int);
145+
create table parted_copytest_a2 (a int, c text, b int);
146+
147+
alter table parted_copytest attach partition parted_copytest_a1 for values in(1);
148+
alter table parted_copytest attach partition parted_copytest_a2 for values in(2);
149+
150+
-- We must insert enough rows to trigger multi-inserts. These are only
151+
-- enabled adaptively when there are few enough partition changes.
152+
insert into parted_copytest select x,1,'One' from generate_series(1,1000) x;
153+
insert into parted_copytest select x,2,'Two' from generate_series(1001,1010) x;
154+
insert into parted_copytest select x,1,'One' from generate_series(1011,1020) x;
155+
156+
copy (select * from parted_copytest order by a) to '@abs_builddir@/results/parted_copytest.csv';
157+
158+
-- Ensure COPY FREEZE errors for partitioned tables.
159+
begin;
160+
truncate parted_copytest;
161+
copy parted_copytest from '@abs_builddir@/results/parted_copytest.csv' (freeze);
162+
rollback;
163+
164+
drop table parted_copytest;

‎src/test/regress/output/copy.source

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,26 @@ copy copytest3 to stdout csv header;
9595
c1,"col with , comma","col with "" quote"
9696
1,a,1
9797
2,b,2
98+
-- test copy from with a partitioned table
99+
create table parted_copytest (
100+
a int,
101+
b int,
102+
c text
103+
) partition by list (b);
104+
create table parted_copytest_a1 (c text, b int, a int);
105+
create table parted_copytest_a2 (a int, c text, b int);
106+
alter table parted_copytest attach partition parted_copytest_a1 for values in(1);
107+
alter table parted_copytest attach partition parted_copytest_a2 for values in(2);
108+
-- We must insert enough rows to trigger multi-inserts. These are only
109+
-- enabled adaptively when there are few enough partition changes.
110+
insert into parted_copytest select x,1,'One' from generate_series(1,1000) x;
111+
insert into parted_copytest select x,2,'Two' from generate_series(1001,1010) x;
112+
insert into parted_copytest select x,1,'One' from generate_series(1011,1020) x;
113+
copy (select * from parted_copytest order by a) to '@abs_builddir@/results/parted_copytest.csv';
114+
-- Ensure COPY FREEZE errors for partitioned tables.
115+
begin;
116+
truncate parted_copytest;
117+
copy parted_copytest from '@abs_builddir@/results/parted_copytest.csv' (freeze);
118+
ERROR: cannot perform FREEZE on a partitioned table
119+
rollback;
120+
drop table parted_copytest;

0 commit comments

Comments
 (0)