Skip to content

Commit aedc460

Browse files
tglsfdctglsfdc
committed
Fix pg_dump --inserts mode for generated columns with dropped columns.
If a table contains a generated column that's preceded by a dropped column, dumpTableData_insert failed to account for the dropped column, and would emit DEFAULT placeholder(s) in the wrong column(s). This resulted in failures at restore time. The default COPY code path did not have this bug, likely explaining why it wasn't noticed sooner. While we're fixing this, we can be a little smarter about the situation: (1) avoid unnecessarily fetching the values of generated columns, (2) omit generated columns from the output, too, if we're using --column-inserts. While these modes aren't expected to be as high-performance as the COPY path, we might as well be as efficient as we can; it doesn't add much complexity. Per report from Дмитрий Иванов. Back-patch to v12 where generated columns came in. Discussion: https://postgr.es/m/CAPL5KHrkBniyQt5e1rafm5DdXvbgiiqfEQEJ9GjtVzN71Jj5pA@mail.gmail.com
1 parent e9af18c commit aedc460

File tree

2 files changed

+111
-9
lines changed

2 files changed

+111
-9
lines changed

‎src/bin/pg_dump/pg_dump.c

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2094,13 +2094,42 @@ dumpTableData_insert(Archive *fout, const void *dcontext)
20942094
DumpOptions *dopt = fout->dopt;
20952095
PQExpBuffer q = createPQExpBuffer();
20962096
PQExpBuffer insertStmt = NULL;
2097+
char *attgenerated;
20972098
PGresult *res;
2098-
int nfields;
2099+
int nfields,
2100+
i;
20992101
int rows_per_statement = dopt->dump_inserts;
21002102
int rows_this_statement = 0;
21012103

2102-
appendPQExpBuffer(q, "DECLARE _pg_dump_cursor CURSOR FOR "
2103-
"SELECT * FROM ONLY %s",
2104+
/*
2105+
* If we're going to emit INSERTs with column names, the most efficient
2106+
* way to deal with generated columns is to exclude them entirely. For
2107+
* INSERTs without column names, we have to emit DEFAULT rather than the
2108+
* actual column value --- but we can save a few cycles by fetching nulls
2109+
* rather than the uninteresting-to-us value.
2110+
*/
2111+
attgenerated = (char *) pg_malloc(tbinfo->numatts * sizeof(char));
2112+
appendPQExpBufferStr(q, "DECLARE _pg_dump_cursor CURSOR FOR SELECT ");
2113+
nfields = 0;
2114+
for (i = 0; i < tbinfo->numatts; i++)
2115+
{
2116+
if (tbinfo->attisdropped[i])
2117+
continue;
2118+
if (tbinfo->attgenerated[i] && dopt->column_inserts)
2119+
continue;
2120+
if (nfields > 0)
2121+
appendPQExpBufferStr(q, ", ");
2122+
if (tbinfo->attgenerated[i])
2123+
appendPQExpBufferStr(q, "NULL");
2124+
else
2125+
appendPQExpBufferStr(q, fmtId(tbinfo->attnames[i]));
2126+
attgenerated[nfields] = tbinfo->attgenerated[i];
2127+
nfields++;
2128+
}
2129+
/* Servers before 9.4 will complain about zero-column SELECT */
2130+
if (nfields == 0)
2131+
appendPQExpBufferStr(q, "NULL");
2132+
appendPQExpBuffer(q, " FROM ONLY %s",
21042133
fmtQualifiedDumpable(tbinfo));
21052134
if (tdinfo->filtercond)
21062135
appendPQExpBuffer(q, " %s", tdinfo->filtercond);
@@ -2111,14 +2140,19 @@ dumpTableData_insert(Archive *fout, const void *dcontext)
21112140
{
21122141
res = ExecuteSqlQuery(fout, "FETCH 100 FROM _pg_dump_cursor",
21132142
PGRES_TUPLES_OK);
2114-
nfields = PQnfields(res);
2143+
2144+
/* cross-check field count, allowing for dummy NULL if any */
2145+
if (nfields != PQnfields(res) &&
2146+
!(nfields == 0 && PQnfields(res) == 1))
2147+
fatal("wrong number of fields retrieved from table \"%s\"",
2148+
tbinfo->dobj.name);
21152149

21162150
/*
21172151
* First time through, we build as much of the INSERT statement as
21182152
* possible in "insertStmt", which we can then just print for each
2119-
* statement. If the table happens to have zero columns then this will
2120-
* be a complete statement, otherwise it will end in "VALUES" and be
2121-
* ready to have the row's column values printed.
2153+
* statement. If the table happens to have zero dumpable columns then
2154+
* this will be a complete statement, otherwise it will end in
2155+
* "VALUES" and be ready to have the row's column values printed.
21222156
*/
21232157
if (insertStmt == NULL)
21242158
{
@@ -2197,7 +2231,7 @@ dumpTableData_insert(Archive *fout, const void *dcontext)
21972231
{
21982232
if (field > 0)
21992233
archputs(", ", fout);
2200-
if (tbinfo->attgenerated[field])
2234+
if (attgenerated[field])
22012235
{
22022236
archputs("DEFAULT", fout);
22032237
continue;
@@ -2302,6 +2336,7 @@ dumpTableData_insert(Archive *fout, const void *dcontext)
23022336
destroyPQExpBuffer(q);
23032337
if (insertStmt != NULL)
23042338
destroyPQExpBuffer(insertStmt);
2339+
free(attgenerated);
23052340

23062341
return 1;
23072342
}

‎src/bin/pg_dump/t/002_pg_dump.pl

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,13 @@
209209
'postgres',
210210
],
211211
},
212+
inserts => {
213+
dump_cmd => [
214+
'pg_dump', '--no-sync',
215+
"--file=$tempdir/inserts.sql", '-a',
216+
'--inserts', 'postgres',
217+
],
218+
},
212219
pg_dumpall_globals => {
213220
dump_cmd => [
214221
'pg_dumpall', '-v', "--file=$tempdir/pg_dumpall_globals.sql",
@@ -603,6 +610,7 @@
603610
%full_runs,
604611
column_inserts => 1,
605612
data_only => 1,
613+
inserts => 1,
606614
section_pre_data => 1,
607615
test_schema_plus_blobs => 1,
608616
},
@@ -930,6 +938,7 @@
930938
%full_runs,
931939
column_inserts => 1,
932940
data_only => 1,
941+
inserts => 1,
933942
section_pre_data => 1,
934943
test_schema_plus_blobs => 1,
935944
},
@@ -950,6 +959,7 @@
950959
%full_runs,
951960
column_inserts => 1,
952961
data_only => 1,
962+
inserts => 1,
953963
section_data => 1,
954964
test_schema_plus_blobs => 1,
955965
},
@@ -1084,6 +1094,7 @@
10841094
%full_runs,
10851095
column_inserts => 1,
10861096
data_only => 1,
1097+
inserts => 1,
10871098
section_pre_data => 1,
10881099
test_schema_plus_blobs => 1,
10891100
},
@@ -1283,6 +1294,27 @@
12831294
},
12841295
},
12851296

1297+
'COPY test_third_table' => {
1298+
create_order => 7,
1299+
create_sql =>
1300+
'INSERT INTO dump_test.test_third_table VALUES (123, DEFAULT, 456);',
1301+
regexp => qr/^
1302+
\QCOPY dump_test.test_third_table (f1, "F3") FROM stdin;\E
1303+
\n123\t456\n\\\.\n
1304+
/xm,
1305+
like => {
1306+
%full_runs,
1307+
%dump_test_schema_runs,
1308+
data_only => 1,
1309+
section_data => 1,
1310+
},
1311+
unlike => {
1312+
binary_upgrade => 1,
1313+
exclude_dump_test_schema => 1,
1314+
schema_only => 1,
1315+
},
1316+
},
1317+
12861318
'COPY test_fourth_table' => {
12871319
create_order => 7,
12881320
create_sql =>
@@ -1374,10 +1406,22 @@
13741406
like => { column_inserts => 1, },
13751407
},
13761408

1409+
'INSERT INTO test_third_table (colnames)' => {
1410+
regexp =>
1411+
qr/^INSERT INTO dump_test\.test_third_table \(f1, "F3"\) VALUES \(123, 456\);\n/m,
1412+
like => { column_inserts => 1, },
1413+
},
1414+
1415+
'INSERT INTO test_third_table' => {
1416+
regexp =>
1417+
qr/^INSERT INTO dump_test\.test_third_table VALUES \(123, DEFAULT, 456, DEFAULT\);\n/m,
1418+
like => { inserts => 1, },
1419+
},
1420+
13771421
'INSERT INTO test_fourth_table' => {
13781422
regexp =>
13791423
qr/^(?:INSERT INTO dump_test\.test_fourth_table DEFAULT VALUES;\n){2}/m,
1380-
like => { column_inserts => 1, rows_per_insert => 1, },
1424+
like => { column_inserts => 1, inserts => 1, rows_per_insert => 1, },
13811425
},
13821426

13831427
'INSERT INTO test_fifth_table' => {
@@ -2547,6 +2591,28 @@
25472591
like => {}
25482592
},
25492593
2594+
'CREATE TABLE test_third_table_generated_cols' => {
2595+
create_order => 6,
2596+
create_sql => 'CREATE TABLE dump_test.test_third_table (
2597+
f1 int, junk int,
2598+
g1 int generated always as (f1 * 2) stored,
2599+
"F3" int,
2600+
g2 int generated always as ("F3" * 3) stored
2601+
);
2602+
ALTER TABLE dump_test.test_third_table DROP COLUMN junk;',
2603+
regexp => qr/^
2604+
\QCREATE TABLE dump_test.test_third_table (\E\n
2605+
\s+\Qf1 integer,\E\n
2606+
\s+\Qg1 integer GENERATED ALWAYS AS ((f1 * 2)) STORED,\E\n
2607+
\s+\Q"F3" integer,\E\n
2608+
\s+\Qg2 integer GENERATED ALWAYS AS (("F3" * 3)) STORED\E\n
2609+
\);\n
2610+
/xm,
2611+
like =>
2612+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2613+
unlike => { binary_upgrade => 1, exclude_dump_test_schema => 1, },
2614+
},
2615+
25502616
'CREATE TABLE test_fourth_table_zero_col' => {
25512617
create_order => 6,
25522618
create_sql => 'CREATE TABLE dump_test.test_fourth_table (
@@ -3230,6 +3296,7 @@
32303296
%full_runs,
32313297
column_inserts => 1,
32323298
data_only => 1,
3299+
inserts => 1,
32333300
section_pre_data => 1,
32343301
test_schema_plus_blobs => 1,
32353302
binary_upgrade => 1,

0 commit comments

Comments
 (0)