Skip to content

Commit 6fc8b14

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 ec383ca commit 6fc8b14

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
@@ -2008,13 +2008,42 @@ dumpTableData_insert(Archive *fout, void *dcontext)
20082008
DumpOptions *dopt = fout->dopt;
20092009
PQExpBuffer q = createPQExpBuffer();
20102010
PQExpBuffer insertStmt = NULL;
2011+
char *attgenerated;
20112012
PGresult *res;
2012-
int nfields;
2013+
int nfields,
2014+
i;
20132015
int rows_per_statement = dopt->dump_inserts;
20142016
int rows_this_statement = 0;
20152017

2016-
appendPQExpBuffer(q, "DECLARE _pg_dump_cursor CURSOR FOR "
2017-
"SELECT * FROM ONLY %s",
2018+
/*
2019+
* If we're going to emit INSERTs with column names, the most efficient
2020+
* way to deal with generated columns is to exclude them entirely. For
2021+
* INSERTs without column names, we have to emit DEFAULT rather than the
2022+
* actual column value --- but we can save a few cycles by fetching nulls
2023+
* rather than the uninteresting-to-us value.
2024+
*/
2025+
attgenerated = (char *) pg_malloc(tbinfo->numatts * sizeof(char));
2026+
appendPQExpBufferStr(q, "DECLARE _pg_dump_cursor CURSOR FOR SELECT ");
2027+
nfields = 0;
2028+
for (i = 0; i < tbinfo->numatts; i++)
2029+
{
2030+
if (tbinfo->attisdropped[i])
2031+
continue;
2032+
if (tbinfo->attgenerated[i] && dopt->column_inserts)
2033+
continue;
2034+
if (nfields > 0)
2035+
appendPQExpBufferStr(q, ", ");
2036+
if (tbinfo->attgenerated[i])
2037+
appendPQExpBufferStr(q, "NULL");
2038+
else
2039+
appendPQExpBufferStr(q, fmtId(tbinfo->attnames[i]));
2040+
attgenerated[nfields] = tbinfo->attgenerated[i];
2041+
nfields++;
2042+
}
2043+
/* Servers before 9.4 will complain about zero-column SELECT */
2044+
if (nfields == 0)
2045+
appendPQExpBufferStr(q, "NULL");
2046+
appendPQExpBuffer(q, " FROM ONLY %s",
20182047
fmtQualifiedDumpable(tbinfo));
20192048
if (tdinfo->filtercond)
20202049
appendPQExpBuffer(q, " %s", tdinfo->filtercond);
@@ -2025,14 +2054,19 @@ dumpTableData_insert(Archive *fout, void *dcontext)
20252054
{
20262055
res = ExecuteSqlQuery(fout, "FETCH 100 FROM _pg_dump_cursor",
20272056
PGRES_TUPLES_OK);
2028-
nfields = PQnfields(res);
2057+
2058+
/* cross-check field count, allowing for dummy NULL if any */
2059+
if (nfields != PQnfields(res) &&
2060+
!(nfields == 0 && PQnfields(res) == 1))
2061+
fatal("wrong number of fields retrieved from table \"%s\"",
2062+
tbinfo->dobj.name);
20292063

20302064
/*
20312065
* First time through, we build as much of the INSERT statement as
20322066
* possible in "insertStmt", which we can then just print for each
2033-
* statement. If the table happens to have zero columns then this will
2034-
* be a complete statement, otherwise it will end in "VALUES" and be
2035-
* ready to have the row's column values printed.
2067+
* statement. If the table happens to have zero dumpable columns then
2068+
* this will be a complete statement, otherwise it will end in
2069+
* "VALUES" and be ready to have the row's column values printed.
20362070
*/
20372071
if (insertStmt == NULL)
20382072
{
@@ -2111,7 +2145,7 @@ dumpTableData_insert(Archive *fout, void *dcontext)
21112145
{
21122146
if (field > 0)
21132147
archputs(", ", fout);
2114-
if (tbinfo->attgenerated[field])
2148+
if (attgenerated[field])
21152149
{
21162150
archputs("DEFAULT", fout);
21172151
continue;
@@ -2216,6 +2250,7 @@ dumpTableData_insert(Archive *fout, void *dcontext)
22162250
destroyPQExpBuffer(q);
22172251
if (insertStmt != NULL)
22182252
destroyPQExpBuffer(insertStmt);
2253+
free(attgenerated);
22192254

22202255
return 1;
22212256
}

‎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",
@@ -584,6 +591,7 @@
584591
%full_runs,
585592
column_inserts => 1,
586593
data_only => 1,
594+
inserts => 1,
587595
section_pre_data => 1,
588596
test_schema_plus_blobs => 1,
589597
},
@@ -911,6 +919,7 @@
911919
%full_runs,
912920
column_inserts => 1,
913921
data_only => 1,
922+
inserts => 1,
914923
section_pre_data => 1,
915924
test_schema_plus_blobs => 1,
916925
},
@@ -931,6 +940,7 @@
931940
%full_runs,
932941
column_inserts => 1,
933942
data_only => 1,
943+
inserts => 1,
934944
section_data => 1,
935945
test_schema_plus_blobs => 1,
936946
},
@@ -1065,6 +1075,7 @@
10651075
%full_runs,
10661076
column_inserts => 1,
10671077
data_only => 1,
1078+
inserts => 1,
10681079
section_pre_data => 1,
10691080
test_schema_plus_blobs => 1,
10701081
},
@@ -1264,6 +1275,27 @@
12641275
},
12651276
},
12661277

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

1390+
'INSERT INTO test_third_table (colnames)' => {
1391+
regexp =>
1392+
qr/^INSERT INTO dump_test\.test_third_table \(f1, "F3"\) VALUES \(123, 456\);\n/m,
1393+
like => { column_inserts => 1, },
1394+
},
1395+
1396+
'INSERT INTO test_third_table' => {
1397+
regexp =>
1398+
qr/^INSERT INTO dump_test\.test_third_table VALUES \(123, DEFAULT, 456, DEFAULT\);\n/m,
1399+
like => { inserts => 1, },
1400+
},
1401+
13581402
'INSERT INTO test_fourth_table' => {
13591403
regexp =>
13601404
qr/^(?:INSERT INTO dump_test\.test_fourth_table DEFAULT VALUES;\n){2}/m,
1361-
like => { column_inserts => 1, rows_per_insert => 1, },
1405+
like => { column_inserts => 1, inserts => 1, rows_per_insert => 1, },
13621406
},
13631407

13641408
'INSERT INTO test_fifth_table' => {
@@ -2483,6 +2527,28 @@
24832527
like => {}
24842528
},
24852529
2530+
'CREATE TABLE test_third_table_generated_cols' => {
2531+
create_order => 6,
2532+
create_sql => 'CREATE TABLE dump_test.test_third_table (
2533+
f1 int, junk int,
2534+
g1 int generated always as (f1 * 2) stored,
2535+
"F3" int,
2536+
g2 int generated always as ("F3" * 3) stored
2537+
);
2538+
ALTER TABLE dump_test.test_third_table DROP COLUMN junk;',
2539+
regexp => qr/^
2540+
\QCREATE TABLE dump_test.test_third_table (\E\n
2541+
\s+\Qf1 integer,\E\n
2542+
\s+\Qg1 integer GENERATED ALWAYS AS ((f1 * 2)) STORED,\E\n
2543+
\s+\Q"F3" integer,\E\n
2544+
\s+\Qg2 integer GENERATED ALWAYS AS (("F3" * 3)) STORED\E\n
2545+
\);\n
2546+
/xm,
2547+
like =>
2548+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2549+
unlike => { binary_upgrade => 1, exclude_dump_test_schema => 1, },
2550+
},
2551+
24862552
'CREATE TABLE test_fourth_table_zero_col' => {
24872553
create_order => 6,
24882554
create_sql => 'CREATE TABLE dump_test.test_fourth_table (
@@ -3152,6 +3218,7 @@
31523218
%full_runs,
31533219
column_inserts => 1,
31543220
data_only => 1,
3221+
inserts => 1,
31553222
section_pre_data => 1,
31563223
test_schema_plus_blobs => 1,
31573224
binary_upgrade => 1,

0 commit comments

Comments
 (0)