Change 'have_error' to 'ret' for simplicity.

schmiddy · schmiddy · commit 673eac7b423e · 2013-06-22T08:59:47.000-04:00
diff --git a/bin/pg_repack.c b/bin/pg_repack.c
@@ -965,7 +965,7 @@ repack_one_table(const repack_table *table, const char *orderby)
 	char		   *vxid = NULL;
 	char			buffer[12];
 	StringInfoData	sql;
-	bool            have_error = false;
+	bool            ret = false;
 
 	/* Keep track of whether we have gotten through setup to install
 	 * the z_repack_trigger, log table, etc. ourselves. We don't want to
@@ -1015,21 +1015,18 @@ repack_one_table(const repack_table *table, const char *orderby)
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
 		elog(ERROR, "%s",  PQerrorMessage(connection));
-		have_error = true;
 		goto cleanup;
 	}
 	else if (strcmp(getstr(res, 0, 0), "t") != 0)
 	{
 		elog(WARNING, "Another pg_repack command may be running on the table. Please try again later.");
-		have_error = true;
 		goto cleanup;
 	}
 	CLEARPGRES(res);
 
 	if (!(lock_exclusive(connection, utoa(table->target_oid, buffer), table->lock_table, TRUE)))
 	{
 		elog(WARNING, "lock_exclusive() failed for %s", table->target_name);
-		have_error = true;
 		goto cleanup;
 	}
 
@@ -1069,7 +1066,6 @@ repack_one_table(const repack_table *table, const char *orderby)
 					PQgetvalue(res, 0, 0), table->target_name)));
 		}
 
-		have_error = true;
 		goto cleanup;
 	}
 	CLEARPGRES(res);
@@ -1108,14 +1104,12 @@ repack_one_table(const repack_table *table, const char *orderby)
 	if (PQsetnonblocking(conn2, 1))
 	{
 		elog(WARNING, "Unable to set conn2 nonblocking.");
-		have_error = true;
 		goto cleanup;
 	}
 	if (!(PQsendQuery(conn2, sql.data)))
 	{
 		elog(WARNING, "Error sending async query: %s\n%s", sql.data,
 			 PQerrorMessage(conn2));
-		have_error = true;
 		goto cleanup;
 	}
 
@@ -1132,7 +1126,6 @@ repack_one_table(const repack_table *table, const char *orderby)
 	if (!(kill_ddl(connection, table->target_oid, true)))
 	{
 		elog(WARNING, "kill_ddl() failed.");
-		have_error = true;
 		goto cleanup;
 	}
 
@@ -1156,7 +1149,6 @@ repack_one_table(const repack_table *table, const char *orderby)
 		if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		{
 			elog(WARNING, "Error with LOCK TABLE: %s", PQerrorMessage(conn2));
-			have_error = true;
 			goto cleanup;
 		}
 		CLEARPGRES(res);
@@ -1166,7 +1158,6 @@ repack_one_table(const repack_table *table, const char *orderby)
 	if (PQsetnonblocking(conn2, 0))
 	{
 		elog(WARNING, "Unable to set conn2 blocking.");
-		have_error = true;
 		goto cleanup;
 	}
 
@@ -1194,7 +1185,6 @@ repack_one_table(const repack_table *table, const char *orderby)
 	{
 		elog(WARNING, "Unable to allocate vxid, length: %d\n",
 			 PQgetlength(res, 0, 0));
-		have_error = true;
 		goto cleanup;
 	}
 	CLEARPGRES(res);
@@ -1219,10 +1209,7 @@ repack_one_table(const repack_table *table, const char *orderby)
 	 * AccessShare lock.
 	 */
 	if (!(lock_access_share(connection, table->target_oid, table->target_name)))
-	{
-		have_error = true;
 		goto cleanup;
-	}
 
 	command(table->create_table, 0, NULL);
 	printfStringInfo(&sql, "SELECT repack.disable_autovacuum('repack.table_%u')", table->target_oid);
@@ -1234,10 +1221,9 @@ repack_one_table(const repack_table *table, const char *orderby)
 	/*
 	 * 3. Create indexes on temp table.
 	 */
-	if (!rebuild_indexes(table)) {
-		have_error = true;
+	if (!rebuild_indexes(table))
 		goto cleanup;
-	}
+
 	CLEARPGRES(res);
 
 	/*
@@ -1294,7 +1280,6 @@ repack_one_table(const repack_table *table, const char *orderby)
 	{
 		elog(WARNING, "lock_exclusive() failed in conn2 for %s",
 			 table->target_name);
-		have_error = true;
 		goto cleanup;
 	}
 
@@ -1330,6 +1315,7 @@ repack_one_table(const repack_table *table, const char *orderby)
 	/* Release advisory lock on table. */
 	res = pgut_execute(connection, "SELECT pg_advisory_unlock($1::bigint)",
 					   1, params);
+	ret = true;
 
 cleanup:
 	CLEARPGRES(res);
@@ -1344,7 +1330,7 @@ repack_one_table(const repack_table *table, const char *orderby)
 	/* XXX: distinguish between fatal and non-fatal errors via the first
 	 * arg to repack_cleanup().
 	 */
-	if (have_error && table_init)
+	if ((!ret) && table_init)
 		repack_cleanup(false, table);
 }
 

Original file line number	Diff line number	Diff line change
`@@ -965,7 +965,7 @@ repack_one_table(const repack_table table, const char orderby)`
`965`	`965`	`char *vxid = NULL;`
`966`	`966`	`char buffer[12];`
`967`	`967`	`StringInfoData sql;`
`968`		`- bool have_error = false;`
	`968`	`+ bool ret = false;`
`969`	`969`
`970`	`970`	`/* Keep track of whether we have gotten through setup to install`
`971`	`971`	`* the z_repack_trigger, log table, etc. ourselves. We don't want to`
`@@ -1015,21 +1015,18 @@ repack_one_table(const repack_table table, const char orderby)`
`1015`	`1015`	`if (PQresultStatus(res) != PGRES_TUPLES_OK)`
`1016`	`1016`	`{`
`1017`	`1017`	`elog(ERROR, "%s", PQerrorMessage(connection));`
`1018`		`- have_error = true;`
`1019`	`1018`	`goto cleanup;`
`1020`	`1019`	`}`
`1021`	`1020`	`else if (strcmp(getstr(res, 0, 0), "t") != 0)`
`1022`	`1021`	`{`
`1023`	`1022`	`elog(WARNING, "Another pg_repack command may be running on the table. Please try again later.");`
`1024`		`- have_error = true;`
`1025`	`1023`	`goto cleanup;`
`1026`	`1024`	`}`
`1027`	`1025`	`CLEARPGRES(res);`
`1028`	`1026`
`1029`	`1027`	`if (!(lock_exclusive(connection, utoa(table->target_oid, buffer), table->lock_table, TRUE)))`
`1030`	`1028`	`{`
`1031`	`1029`	`elog(WARNING, "lock_exclusive() failed for %s", table->target_name);`
`1032`		`- have_error = true;`
`1033`	`1030`	`goto cleanup;`
`1034`	`1031`	`}`
`1035`	`1032`
`@@ -1069,7 +1066,6 @@ repack_one_table(const repack_table table, const char orderby)`
`1069`	`1066`	`PQgetvalue(res, 0, 0), table->target_name)));`
`1070`	`1067`	`}`
`1071`	`1068`
`1072`		`- have_error = true;`
`1073`	`1069`	`goto cleanup;`
`1074`	`1070`	`}`
`1075`	`1071`	`CLEARPGRES(res);`
`@@ -1108,14 +1104,12 @@ repack_one_table(const repack_table table, const char orderby)`
`1108`	`1104`	`if (PQsetnonblocking(conn2, 1))`
`1109`	`1105`	`{`
`1110`	`1106`	`elog(WARNING, "Unable to set conn2 nonblocking.");`
`1111`		`- have_error = true;`
`1112`	`1107`	`goto cleanup;`
`1113`	`1108`	`}`
`1114`	`1109`	`if (!(PQsendQuery(conn2, sql.data)))`
`1115`	`1110`	`{`
`1116`	`1111`	`elog(WARNING, "Error sending async query: %s\n%s", sql.data,`
`1117`	`1112`	`PQerrorMessage(conn2));`
`1118`		`- have_error = true;`
`1119`	`1113`	`goto cleanup;`
`1120`	`1114`	`}`
`1121`	`1115`
`@@ -1132,7 +1126,6 @@ repack_one_table(const repack_table table, const char orderby)`
`1132`	`1126`	`if (!(kill_ddl(connection, table->target_oid, true)))`
`1133`	`1127`	`{`
`1134`	`1128`	`elog(WARNING, "kill_ddl() failed.");`
`1135`		`- have_error = true;`
`1136`	`1129`	`goto cleanup;`
`1137`	`1130`	`}`
`1138`	`1131`
`@@ -1156,7 +1149,6 @@ repack_one_table(const repack_table table, const char orderby)`
`1156`	`1149`	`if (PQresultStatus(res) != PGRES_COMMAND_OK)`
`1157`	`1150`	`{`
`1158`	`1151`	`elog(WARNING, "Error with LOCK TABLE: %s", PQerrorMessage(conn2));`
`1159`		`- have_error = true;`
`1160`	`1152`	`goto cleanup;`
`1161`	`1153`	`}`
`1162`	`1154`	`CLEARPGRES(res);`
`@@ -1166,7 +1158,6 @@ repack_one_table(const repack_table table, const char orderby)`
`1166`	`1158`	`if (PQsetnonblocking(conn2, 0))`
`1167`	`1159`	`{`
`1168`	`1160`	`elog(WARNING, "Unable to set conn2 blocking.");`
`1169`		`- have_error = true;`
`1170`	`1161`	`goto cleanup;`
`1171`	`1162`	`}`
`1172`	`1163`
`@@ -1194,7 +1185,6 @@ repack_one_table(const repack_table table, const char orderby)`
`1194`	`1185`	`{`
`1195`	`1186`	`elog(WARNING, "Unable to allocate vxid, length: %d\n",`
`1196`	`1187`	`PQgetlength(res, 0, 0));`
`1197`		`- have_error = true;`
`1198`	`1188`	`goto cleanup;`
`1199`	`1189`	`}`
`1200`	`1190`	`CLEARPGRES(res);`
`@@ -1219,10 +1209,7 @@ repack_one_table(const repack_table table, const char orderby)`
`1219`	`1209`	`* AccessShare lock.`
`1220`	`1210`	`*/`
`1221`	`1211`	`if (!(lock_access_share(connection, table->target_oid, table->target_name)))`
`1222`		`- {`
`1223`		`- have_error = true;`
`1224`	`1212`	`goto cleanup;`
`1225`		`- }`
`1226`	`1213`
`1227`	`1214`	`command(table->create_table, 0, NULL);`
`1228`	`1215`	`printfStringInfo(&sql, "SELECT repack.disable_autovacuum('repack.table_%u')", table->target_oid);`
`@@ -1234,10 +1221,9 @@ repack_one_table(const repack_table table, const char orderby)`
`1234`	`1221`	`/*`
`1235`	`1222`	`* 3. Create indexes on temp table.`
`1236`	`1223`	`*/`
`1237`		`- if (!rebuild_indexes(table)) {`
`1238`		`- have_error = true;`
	`1224`	`+ if (!rebuild_indexes(table))`
`1239`	`1225`	`goto cleanup;`
`1240`		`- }`
	`1226`	`+`
`1241`	`1227`	`CLEARPGRES(res);`
`1242`	`1228`
`1243`	`1229`	`/*`
`@@ -1294,7 +1280,6 @@ repack_one_table(const repack_table table, const char orderby)`
`1294`	`1280`	`{`
`1295`	`1281`	`elog(WARNING, "lock_exclusive() failed in conn2 for %s",`
`1296`	`1282`	`table->target_name);`
`1297`		`- have_error = true;`
`1298`	`1283`	`goto cleanup;`
`1299`	`1284`	`}`
`1300`	`1285`
`@@ -1330,6 +1315,7 @@ repack_one_table(const repack_table table, const char orderby)`
`1330`	`1315`	`/* Release advisory lock on table. */`
`1331`	`1316`	`res = pgut_execute(connection, "SELECT pg_advisory_unlock($1::bigint)",`
`1332`	`1317`	`1, params);`
	`1318`	`+ ret = true;`
`1333`	`1319`
`1334`	`1320`	`cleanup:`
`1335`	`1321`	`CLEARPGRES(res);`
`@@ -1344,7 +1330,7 @@ repack_one_table(const repack_table table, const char orderby)`
`1344`	`1330`	`/* XXX: distinguish between fatal and non-fatal errors via the first`
`1345`	`1331`	`* arg to repack_cleanup().`
`1346`	`1332`	`*/`
`1347`		`- if (have_error && table_init)`
	`1333`	`+ if ((!ret) && table_init)`
`1348`	`1334`	`repack_cleanup(false, table);`
`1349`	`1335`	`}`
`1350`	`1336`