Skip to content

Commit 6155932

Browse files
ItGackyItGacky
committed
Fix data corruption bug reported by robjderr (#1010664).
pg_reorg broke catalog definition if the target table had any dropped columns. Now pg_reorg removes dropped columns and renumbers valid columns. You can use pg_reorg to shrink column definitions if you have many dropped columns. (without pg_reorg, dropped columns are filled with zero forever)
1 parent 5fe3f03 commit 6155932

File tree

10 files changed

+252
-137
lines changed

10 files changed

+252
-137
lines changed

‎bin/expected/reorg.out

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ CREATE TABLE tbl_cluster (
66
col1 int,
77
col2 timestamp,
88
":-)" text,
9-
primary key(":-)", col1)
9+
PRIMARY KEY (":-)", col1)
1010
) WITH (fillfactor = 70);
1111
CREATE INDEX cidx_cluster ON tbl_cluster (col2, length(":-)"));
1212
ALTER TABLE tbl_cluster CLUSTER ON cidx_cluster;
@@ -27,6 +27,17 @@ CREATE TABLE tbl_gistkey (
2727
);
2828
CREATE INDEX cidx_circle ON tbl_gistkey USING gist (c);
2929
ALTER TABLE tbl_gistkey CLUSTER ON cidx_circle;
30+
CREATE TABLE tbl_with_dropped_column (
31+
d1 text,
32+
c1 text,
33+
id integer PRIMARY KEY,
34+
d2 text,
35+
c2 text,
36+
d3 text
37+
);
38+
ALTER TABLE tbl_with_dropped_column CLUSTER ON tbl_with_dropped_column_pkey;
39+
CREATE INDEX idx_c1c2 ON tbl_with_dropped_column (c1, c2);
40+
CREATE INDEX idx_c2c1 ON tbl_with_dropped_column (c2, c1);
3041
--
3142
-- insert data
3243
--
@@ -41,14 +52,30 @@ INSERT INTO tbl_only_ckey VALUES(1, '2008-01-01 00:00:00', 'abc');
4152
INSERT INTO tbl_only_ckey VALUES(2, '2008-02-01 00:00:00', 'def');
4253
INSERT INTO tbl_gistkey VALUES(1, '<(1,2),3>');
4354
INSERT INTO tbl_gistkey VALUES(2, '<(4,5),6>');
55+
INSERT INTO tbl_with_dropped_column VALUES('d1', 'c1', 2, 'd2', 'c2', 'd3');
56+
INSERT INTO tbl_with_dropped_column VALUES('d1', 'c1', 1, 'd2', 'c2', 'd3');
57+
ALTER TABLE tbl_with_dropped_column DROP COLUMN d1;
58+
ALTER TABLE tbl_with_dropped_column DROP COLUMN d2;
59+
ALTER TABLE tbl_with_dropped_column DROP COLUMN d3;
60+
ALTER TABLE tbl_with_dropped_column ADD COLUMN c3 text;
61+
--
62+
-- before
63+
--
64+
SELECT * FROM tbl_with_dropped_column;
65+
c1 | id | c2 | c3
66+
----+----+----+----
67+
c1 | 2 | c2 |
68+
c1 | 1 | c2 |
69+
(2 rows)
70+
4471
--
4572
-- do reorg
4673
--
4774
\! pg_reorg --dbname=contrib_regression --no-order
4875
\! pg_reorg --dbname=contrib_regression
4976
\! pg_reorg --dbname=contrib_regression --table=tbl_cluster
5077
--
51-
-- results
78+
-- after
5279
--
5380
\d tbl_cluster
5481
Table "public.tbl_cluster"
@@ -90,6 +117,19 @@ Indexes:
90117
Indexes:
91118
"tbl_only_pkey_pkey" PRIMARY KEY, btree (col1)
92119

120+
\d tbl_with_dropped_column
121+
Table "public.tbl_with_dropped_column"
122+
Column | Type | Modifiers
123+
--------+---------+-----------
124+
c1 | text |
125+
id | integer | not null
126+
c2 | text |
127+
c3 | text |
128+
Indexes:
129+
"tbl_with_dropped_column_pkey" PRIMARY KEY, btree (id) CLUSTER
130+
"idx_c1c2" btree (c1, c2)
131+
"idx_c2c1" btree (c2, c1)
132+
93133
SELECT col1, to_char(col2, 'YYYY-MM-DD HH24:MI:SS'), ":-)" FROM tbl_cluster;
94134
col1 | to_char | :-)
95135
------+---------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -121,11 +161,19 @@ SELECT * FROM tbl_gistkey ORDER BY 1;
121161
2 | <(4,5),6>
122162
(2 rows)
123163

164+
SELECT * FROM tbl_with_dropped_column;
165+
c1 | id | c2 | c3
166+
----+----+----+----
167+
c1 | 1 | c2 |
168+
c1 | 2 | c2 |
169+
(2 rows)
170+
124171
--
125172
-- clean up
126173
--
127174
DROP TABLE tbl_cluster;
128175
DROP TABLE tbl_only_pkey;
129176
DROP TABLE tbl_only_ckey;
130177
DROP TABLE tbl_gistkey;
178+
DROP TABLE tbl_with_dropped_column;
131179
RESET client_min_messages;

‎bin/pg_reorg.c

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* @brief Client Modules
99
*/
1010

11-
const char *PROGRAM_VERSION = "1.0.5";
11+
const char *PROGRAM_VERSION = "1.0.6";
1212
const char *PROGRAM_URL = "http://reorg.projects.postgresql.org/";
1313
const char *PROGRAM_EMAIL = "reorg-general@lists.pgfoundry.org";
1414

@@ -93,7 +93,6 @@ static bool sqlstate_equals(PGresult *res, const char *state)
9393
}
9494

9595
static bool verbose = false;
96-
static bool quiet = false;
9796
static bool analyze = true;
9897

9998
/*
@@ -111,7 +110,6 @@ utoa(unsigned int value, char *buffer)
111110
}
112111

113112
const struct option pgut_options[] = {
114-
{"quiet", no_argument, NULL, 'q'},
115113
{"verbose", no_argument, NULL, 'v'},
116114
{"all", no_argument, NULL, 'a'},
117115
{"table", required_argument, NULL, 't'},
@@ -130,9 +128,6 @@ pgut_argument(int c, const char *arg)
130128
{
131129
switch (c)
132130
{
133-
case 'q':
134-
quiet = true;
135-
break;
136131
case 'v':
137132
verbose = true;
138133
break;
@@ -587,14 +582,17 @@ reorg_one_table(const reorg_table *table, const char *orderby)
587582
* Note that current_table is already set to NULL here because analyze
588583
* is an unimportant operation; No clean up even if failed.
589584
*/
590-
if (verbose)
591-
fprintf(stderr, "---- analyze ----\n");
585+
if (analyze)
586+
{
587+
if (verbose)
588+
fprintf(stderr, "---- analyze ----\n");
592589

593-
command("BEGIN ISOLATION LEVEL READ COMMITTED", 0, NULL);
594-
printfStringInfo(&sql, "ANALYZE %s%s",
595-
(verbose ? "VERBOSE " : ""), table->target_name);
596-
command(sql.data, 0, NULL);
597-
command("COMMIT", 0, NULL);
590+
command("BEGIN ISOLATION LEVEL READ COMMITTED", 0, NULL);
591+
printfStringInfo(&sql, "ANALYZE %s%s",
592+
(verbose ? "VERBOSE " : ""), table->target_name);
593+
command(sql.data, 0, NULL);
594+
command("COMMIT", 0, NULL);
595+
}
598596

599597
termStringInfo(&sql);
600598
}
@@ -643,7 +641,6 @@ pgut_help(void)
643641
" -n, --no-order do vacuum full instead of cluster\n"
644642
" -o, --order-by=columns order by columns instead of cluster keys\n"
645643
" -Z, --no-analyze don't analyze at end\n"
646-
" -q, --quiet don't write any messages\n"
647644
" -v, --verbose display detailed information during processing\n",
648645
PROGRAM_NAME, PROGRAM_NAME);
649646
}

‎bin/pgut/pgut.c

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const char *port = NULL;
2222
const char *username = NULL;
2323
bool password = false;
2424
bool debug = false;
25+
bool quiet = false;
2526

2627
/* Database connections */
2728
PGconn *connection = NULL;
@@ -45,6 +46,7 @@ const struct option default_options[] =
4546
{"dbname", required_argument, NULL, 'd'},
4647
{"host", required_argument, NULL, 'h'},
4748
{"port", required_argument, NULL, 'p'},
49+
{"quiet", no_argument, NULL, 'q'},
4850
{"username", required_argument, NULL, 'U'},
4951
{"password", no_argument, NULL, 'W'},
5052
{"debug", no_argument, NULL, '!'},
@@ -136,6 +138,9 @@ parse_options(int argc, char **argv)
136138
case 'p':
137139
assign_option(&port, c, optarg);
138140
break;
141+
case 'q':
142+
quiet = true;
143+
break;
139144
case 'U':
140145
assign_option(&username, c, optarg);
141146
break;
@@ -196,13 +201,22 @@ assign_option(const char **value, int c, const char *arg)
196201
return true;
197202
}
198203

199-
void
200-
reconnect(void)
204+
/*
205+
* the result is also available with the global variable 'connection'.
206+
*/
207+
PGconn *
208+
reconnect_elevel(int elevel)
201209
{
202210
PGconn *conn;
203211
char *pwd = NULL;
204212
bool new_pass;
205213

214+
if (interrupted)
215+
{
216+
interrupted = false;
217+
elog(ERROR, "%s: interrupted", PROGRAM_NAME);
218+
}
219+
206220
disconnect();
207221

208222
if (password)
@@ -218,7 +232,10 @@ reconnect(void)
218232
conn = PQsetdbLogin(host, port, NULL, NULL, dbname, username, pwd);
219233

220234
if (!conn)
221-
elog(ERROR, "could not connect to database %s", dbname);
235+
{
236+
elog(elevel, "could not connect to database %s", dbname);
237+
return NULL;
238+
}
222239

223240
if (PQstatus(conn) == CONNECTION_BAD &&
224241
#if PG_VERSION_NUM >= 80300
@@ -239,10 +256,17 @@ reconnect(void)
239256

240257
/* check to see that the backend connection was successfully made */
241258
if (PQstatus(conn) == CONNECTION_BAD)
242-
elog(ERROR, "could not connect to database %s: %s",
259+
elog(elevel, "could not connect to database %s: %s",
243260
dbname, PQerrorMessage(conn));
244261

245262
connection = conn;
263+
return conn;
264+
}
265+
266+
void
267+
reconnect(void)
268+
{
269+
reconnect_elevel(ERROR);
246270
}
247271

248272
void
@@ -290,6 +314,7 @@ execute_elevel(const char *query, int nParams, const char **params, int elevel)
290314
{
291315
case PGRES_TUPLES_OK:
292316
case PGRES_COMMAND_OK:
317+
case PGRES_COPY_IN:
293318
break;
294319
default:
295320
elog(elevel, "query failed: %squery was: %s",
@@ -329,6 +354,8 @@ elog(int elevel, const char *fmt, ...)
329354

330355
if (!debug && elevel <= LOG)
331356
return;
357+
if (quiet && elevel <= WARNING)
358+
return;
332359

333360
switch (elevel)
334361
{
@@ -481,6 +508,7 @@ static void help(void)
481508
fprintf(stderr, " -U, --username=USERNAME user name to connect as\n");
482509
fprintf(stderr, " -W, --password force password prompt\n");
483510
fprintf(stderr, "\nGeneric options:\n");
511+
fprintf(stderr, " -q, --quiet don't write any messages\n");
484512
fprintf(stderr, " --debug debug mode\n");
485513
fprintf(stderr, " --help show this help, then exit\n");
486514
fprintf(stderr, " --version output version information, then exit\n\n");
@@ -566,3 +594,4 @@ sleep(unsigned int seconds)
566594
}
567595

568596
#endif /* WIN32 */
597+

‎bin/pgut/pgut.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "libpq-fe.h"
1414
#include "pqexpbuffer.h"
1515

16+
#include <assert.h>
1617
#include <getopt.h>
1718

1819
#if !defined(C_H) && !defined(__cplusplus)
@@ -51,13 +52,16 @@ extern const char *port;
5152
extern const char *username;
5253
extern bool password;
5354
extern bool debug;
55+
extern bool quiet;
5456

5557
extern PGconn *connection;
5658
extern bool interrupted;
5759

5860
extern void parse_options(int argc, char **argv);
5961
extern bool assign_option(const char **value, int c, const char *arg);
6062

63+
64+
extern PGconn *reconnect_elevel(int elevel);
6165
extern void reconnect(void);
6266
extern void disconnect(void);
6367
extern PGresult *execute_elevel(const char *query, int nParams, const char **params, int elevel);
@@ -68,6 +72,15 @@ extern void command(const char *query, int nParams, const char **params);
6872
extern unsigned int sleep(unsigned int seconds);
6973
#endif
7074

75+
/*
76+
* IsXXX
77+
*/
78+
#define IsSpace(c) (isspace((unsigned char)(c)))
79+
#define IsAlpha(c) (isalpha((unsigned char)(c)))
80+
#define IsAlnum(c) (isalnum((unsigned char)(c)))
81+
#define IsIdentHead(c) (IsAlpha(c) || (c) == '_')
82+
#define IsIdentBody(c) (IsAlnum(c) || (c) == '_')
83+
7184
/*
7285
* elog
7386
*/
@@ -104,6 +117,20 @@ __attribute__((format(printf, 2, 3)));
104117
#define appendStringInfoChar appendPQExpBufferChar
105118
#define appendBinaryStringInfo appendBinaryPQExpBuffer
106119

120+
/*
121+
* Assert
122+
*/
123+
#undef Assert
124+
#undef AssertMacro
125+
126+
#ifdef USE_ASSERT_CHECKING
127+
#define Assert(x) assert(x)
128+
#define AssertMacro(x) assert(x)
129+
#else
130+
#define Assert(x) ((void) 0)
131+
#define AssertMacro(x) ((void) 0)
132+
#endif
133+
107134
/*
108135
* import from postgres.h and catalog/genbki.h in 8.4
109136
*/
@@ -124,3 +151,4 @@ typedef int aclitem;
124151
#endif
125152

126153
#endif /* PGUT_H */
154+

0 commit comments

Comments
 (0)