Skip to content

Commit 1e7f588

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 0416c9c commit 1e7f588

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
@@ -1949,13 +1949,42 @@ dumpTableData_insert(Archive *fout, void *dcontext)
19491949
DumpOptions *dopt = fout->dopt;
19501950
PQExpBuffer q = createPQExpBuffer();
19511951
PQExpBuffer insertStmt = NULL;
1952+
char *attgenerated;
19521953
PGresult *res;
1953-
int nfields;
1954+
int nfields,
1955+
i;
19541956
int rows_per_statement = dopt->dump_inserts;
19551957
int rows_this_statement = 0;
19561958

1957-
appendPQExpBuffer(q, "DECLARE _pg_dump_cursor CURSOR FOR "
1958-
"SELECT * FROM ONLY %s",
1959+
/*
1960+
* If we're going to emit INSERTs with column names, the most efficient
1961+
* way to deal with generated columns is to exclude them entirely. For
1962+
* INSERTs without column names, we have to emit DEFAULT rather than the
1963+
* actual column value --- but we can save a few cycles by fetching nulls
1964+
* rather than the uninteresting-to-us value.
1965+
*/
1966+
attgenerated = (char *) pg_malloc(tbinfo->numatts * sizeof(char));
1967+
appendPQExpBufferStr(q, "DECLARE _pg_dump_cursor CURSOR FOR SELECT ");
1968+
nfields = 0;
1969+
for (i = 0; i < tbinfo->numatts; i++)
1970+
{
1971+
if (tbinfo->attisdropped[i])
1972+
continue;
1973+
if (tbinfo->attgenerated[i] && dopt->column_inserts)
1974+
continue;
1975+
if (nfields > 0)
1976+
appendPQExpBufferStr(q, ", ");
1977+
if (tbinfo->attgenerated[i])
1978+
appendPQExpBufferStr(q, "NULL");
1979+
else
1980+
appendPQExpBufferStr(q, fmtId(tbinfo->attnames[i]));
1981+
attgenerated[nfields] = tbinfo->attgenerated[i];
1982+
nfields++;
1983+
}
1984+
/* Servers before 9.4 will complain about zero-column SELECT */
1985+
if (nfields == 0)
1986+
appendPQExpBufferStr(q, "NULL");
1987+
appendPQExpBuffer(q, " FROM ONLY %s",
19591988
fmtQualifiedDumpable(tbinfo));
19601989
if (tdinfo->filtercond)
19611990
appendPQExpBuffer(q, " %s", tdinfo->filtercond);
@@ -1966,14 +1995,19 @@ dumpTableData_insert(Archive *fout, void *dcontext)
19661995
{
19671996
res = ExecuteSqlQuery(fout, "FETCH 100 FROM _pg_dump_cursor",
19681997
PGRES_TUPLES_OK);
1969-
nfields = PQnfields(res);
1998+
1999+
/* cross-check field count, allowing for dummy NULL if any */
2000+
if (nfields != PQnfields(res) &&
2001+
!(nfields == 0 && PQnfields(res) == 1))
2002+
fatal("wrong number of fields retrieved from table \"%s\"",
2003+
tbinfo->dobj.name);
19702004

19712005
/*
19722006
* First time through, we build as much of the INSERT statement as
19732007
* possible in "insertStmt", which we can then just print for each
1974-
* statement. If the table happens to have zero columns then this will
1975-
* be a complete statement, otherwise it will end in "VALUES" and be
1976-
* ready to have the row's column values printed.
2008+
* statement. If the table happens to have zero dumpable columns then
2009+
* this will be a complete statement, otherwise it will end in
2010+
* "VALUES" and be ready to have the row's column values printed.
19772011
*/
19782012
if (insertStmt == NULL)
19792013
{
@@ -2052,7 +2086,7 @@ dumpTableData_insert(Archive *fout, void *dcontext)
20522086
{
20532087
if (field > 0)
20542088
archputs(", ", fout);
2055-
if (tbinfo->attgenerated[field])
2089+
if (attgenerated[field])
20562090
{
20572091
archputs("DEFAULT", fout);
20582092
continue;
@@ -2157,6 +2191,7 @@ dumpTableData_insert(Archive *fout, void *dcontext)
21572191
destroyPQExpBuffer(q);
21582192
if (insertStmt != NULL)
21592193
destroyPQExpBuffer(insertStmt);
2194+
free(attgenerated);
21602195

21612196
return 1;
21622197
}

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

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,13 @@
206206
'postgres',
207207
],
208208
},
209+
inserts => {
210+
dump_cmd => [
211+
'pg_dump', '--no-sync',
212+
"--file=$tempdir/inserts.sql", '-a',
213+
'--inserts', 'postgres',
214+
],
215+
},
209216
pg_dumpall_globals => {
210217
dump_cmd => [
211218
'pg_dumpall', '-v', "--file=$tempdir/pg_dumpall_globals.sql",
@@ -582,6 +589,7 @@
582589
%full_runs,
583590
column_inserts => 1,
584591
data_only => 1,
592+
inserts => 1,
585593
section_pre_data => 1,
586594
test_schema_plus_blobs => 1,
587595
},
@@ -909,6 +917,7 @@
909917
%full_runs,
910918
column_inserts => 1,
911919
data_only => 1,
920+
inserts => 1,
912921
section_pre_data => 1,
913922
test_schema_plus_blobs => 1,
914923
},
@@ -929,6 +938,7 @@
929938
%full_runs,
930939
column_inserts => 1,
931940
data_only => 1,
941+
inserts => 1,
932942
section_data => 1,
933943
test_schema_plus_blobs => 1,
934944
},
@@ -1063,6 +1073,7 @@
10631073
%full_runs,
10641074
column_inserts => 1,
10651075
data_only => 1,
1076+
inserts => 1,
10661077
section_pre_data => 1,
10671078
test_schema_plus_blobs => 1,
10681079
},
@@ -1262,6 +1273,27 @@
12621273
},
12631274
},
12641275

1276+
'COPY test_third_table' => {
1277+
create_order => 7,
1278+
create_sql =>
1279+
'INSERT INTO dump_test.test_third_table VALUES (123, DEFAULT, 456);',
1280+
regexp => qr/^
1281+
\QCOPY dump_test.test_third_table (f1, "F3") FROM stdin;\E
1282+
\n123\t456\n\\\.\n
1283+
/xm,
1284+
like => {
1285+
%full_runs,
1286+
%dump_test_schema_runs,
1287+
data_only => 1,
1288+
section_data => 1,
1289+
},
1290+
unlike => {
1291+
binary_upgrade => 1,
1292+
exclude_dump_test_schema => 1,
1293+
schema_only => 1,
1294+
},
1295+
},
1296+
12651297
'COPY test_fourth_table' => {
12661298
create_order => 7,
12671299
create_sql =>
@@ -1353,10 +1385,22 @@
13531385
like => { column_inserts => 1, },
13541386
},
13551387

1388+
'INSERT INTO test_third_table (colnames)' => {
1389+
regexp =>
1390+
qr/^INSERT INTO dump_test\.test_third_table \(f1, "F3"\) VALUES \(123, 456\);\n/m,
1391+
like => { column_inserts => 1, },
1392+
},
1393+
1394+
'INSERT INTO test_third_table' => {
1395+
regexp =>
1396+
qr/^INSERT INTO dump_test\.test_third_table VALUES \(123, DEFAULT, 456, DEFAULT\);\n/m,
1397+
like => { inserts => 1, },
1398+
},
1399+
13561400
'INSERT INTO test_fourth_table' => {
13571401
regexp =>
13581402
qr/^(?:INSERT INTO dump_test\.test_fourth_table DEFAULT VALUES;\n){2}/m,
1359-
like => { column_inserts => 1, rows_per_insert => 1, },
1403+
like => { column_inserts => 1, inserts => 1, rows_per_insert => 1, },
13601404
},
13611405

13621406
'INSERT INTO test_fifth_table' => {
@@ -2469,6 +2513,28 @@
24692513
like => {}
24702514
},
24712515
2516+
'CREATE TABLE test_third_table_generated_cols' => {
2517+
create_order => 6,
2518+
create_sql => 'CREATE TABLE dump_test.test_third_table (
2519+
f1 int, junk int,
2520+
g1 int generated always as (f1 * 2) stored,
2521+
"F3" int,
2522+
g2 int generated always as ("F3" * 3) stored
2523+
);
2524+
ALTER TABLE dump_test.test_third_table DROP COLUMN junk;',
2525+
regexp => qr/^
2526+
\QCREATE TABLE dump_test.test_third_table (\E\n
2527+
\s+\Qf1 integer,\E\n
2528+
\s+\Qg1 integer GENERATED ALWAYS AS ((f1 * 2)) STORED,\E\n
2529+
\s+\Q"F3" integer,\E\n
2530+
\s+\Qg2 integer GENERATED ALWAYS AS (("F3" * 3)) STORED\E\n
2531+
\);\n
2532+
/xm,
2533+
like =>
2534+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2535+
unlike => { binary_upgrade => 1, exclude_dump_test_schema => 1, },
2536+
},
2537+
24722538
'CREATE TABLE test_fourth_table_zero_col' => {
24732539
create_order => 6,
24742540
create_sql => 'CREATE TABLE dump_test.test_fourth_table (
@@ -3126,6 +3192,7 @@
31263192
%full_runs,
31273193
column_inserts => 1,
31283194
data_only => 1,
3195+
inserts => 1,
31293196
section_pre_data => 1,
31303197
test_schema_plus_blobs => 1,
31313198
binary_upgrade => 1,

0 commit comments

Comments
 (0)