Skip to content

Commit 62b5e4f

Browse files
schmiddyschmiddy
committed
A few cosmetic fixes to pg_repack.c:
 * fix old reference to --indexes-only * trailing whitespace and indentation cleanup to match rest of code * XXX note about improving advisory locking
1 parent b804c15 commit 62b5e4f

File tree

1 file changed

+46
-32
lines changed

1 file changed

+46
-32
lines changed

‎bin/pg_repack.c

Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ main(int argc, char *argv[])
257257
errmsg("cannot specify --index (-i) and --table (-t)")));
258258
else if (r_index && only_indexes)
259259
ereport(ERROR, (errcode(EINVAL),
260-
errmsg("cannot specify --index (-i) and --indexes_only (-x)")));
260+
errmsg("cannot specify --index (-i) and --only-index (-x)")));
261261
else if (only_indexes && !table_list.head)
262262
ereport(ERROR, (errcode(EINVAL),
263263
errmsg("cannot repack all indexes of database, specify the table with -t option")));
@@ -969,7 +969,7 @@ repack_one_table(const repack_table *table, const char *orderby)
969969

970970
/* Keep track of whether we have gotten through setup to install
971971
* the z_repack_trigger, log table, etc. ourselves. We don't want to
972-
* go through repack_cleanup() if we didnt' actually set up the
972+
* go through repack_cleanup() if we didn't actually set up the
973973
* trigger ourselves, lest we be cleaning up another pg_repack's mess,
974974
* or worse, interfering with a still-running pg_repack.
975975
*/
@@ -1012,7 +1012,7 @@ repack_one_table(const repack_table *table, const char *orderby)
10121012
}
10131013

10141014
/*
1015-
* Check z_repack_trigger is the trigger executed at last so that
1015+
* Check z_repack_trigger is the trigger executed last so that
10161016
* other before triggers cannot modify triggered tuples.
10171017
*/
10181018
params[0] = utoa(table->target_oid, buffer);
@@ -1577,11 +1577,12 @@ repack_cleanup(bool fatal, const repack_table *table)
15771577
/*
15781578
* repack one index
15791579
*/
1580-
static bool
1581-
repack_one_index(Oid table, const char *table_name, Oid index, const char *schema_name){
1580+
static bool
1581+
repack_one_index(Oid table, const char *table_name, Oid index, const char *schema_name)
1582+
{
15821583
bool ret = false;
15831584
PGresult *res = NULL;
1584-
StringInfoData sql, temp_index;
1585+
StringInfoData sql, temp_index;
15851586
char buffer[2][12];
15861587
char *create_idx;
15871588
const char *params[3];
@@ -1593,13 +1594,13 @@ repack_one_index(Oid table, const char *table_name, Oid index, const char *schem
15931594
if (PQntuples(res) < 1)
15941595
{
15951596
ereport(ERROR, (errcode(EINVAL),
1596-
errmsg("unable to generate SQL to CREATE new index")));
1597+
errmsg("unable to generate SQL to CREATE new index")));
15971598
goto cleanup;
15981599
}
15991600
create_idx = getstr(res, 0, 0);
16001601
CLEARPGRES(res);
16011602
res = execute_elevel(create_idx, 0, NULL, DEBUG2);
1602-
1603+
16031604
initStringInfo(&temp_index);
16041605
if (schema_name)
16051606
appendStringInfo(&temp_index, "%s.index_%u", schema_name, index);
@@ -1611,25 +1612,27 @@ repack_one_index(Oid table, const char *table_name, Oid index, const char *schem
16111612
ereport(ERROR,
16121613
(errcode(E_PG_COMMAND),
16131614
errmsg("%s", PQerrorMessage(connection)),
1614-
errdetail("The temporary index may be left behind "
1615-
" by a pg_repack command on the table which"
1616-
" was interrupted and failed to clean up"
1617-
" the temporary objects. Please use the \"DROP INDEX %s\""
1618-
" to remove the temporary index.", temp_index.data)));
1615+
errdetail("An invalid index may have been left behind "
1616+
" by a pg_repack command on the table which"
1617+
" was interrupted and failed to clean up"
1618+
" the temporary objects. Please use \"DROP INDEX %s\""
1619+
" to remove this index.", temp_index.data)));
16191620
goto cleanup;
16201621
}
16211622
CLEARPGRES(res);
16221623

16231624
/* take exclusive lock on table before calling repack_index_swap() */
16241625
initStringInfo(&sql);
16251626
if (schema_name)
1626-
appendStringInfo(&sql, "LOCK TABLE %s.%s IN ACCESS EXCLUSIVE MODE", schema_name, table_name);
1627+
appendStringInfo(&sql, "LOCK TABLE %s.%s IN ACCESS EXCLUSIVE MODE",
1628+
schema_name, table_name);
16271629
else
1628-
appendStringInfo(&sql, "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE", table_name);
1630+
appendStringInfo(&sql, "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE",
1631+
table_name);
16291632
if (!(lock_exclusive(connection, params[1], sql.data, TRUE)))
16301633
{
16311634
elog(WARNING, "lock_exclusive() failed in connection for %s",
1632-
table_name);
1635+
table_name);
16331636
goto drop_idx;
16341637
}
16351638
pgut_command(connection, "SELECT repack.repack_index_swap($1)", 1, params);
@@ -1645,6 +1648,7 @@ repack_one_index(Oid table, const char *table_name, Oid index, const char *schem
16451648
appendStringInfo(&sql, "%s", temp_index.data);
16461649
command(sql.data, 0, NULL);
16471650
ret = true;
1651+
16481652
cleanup:
16491653
CLEARPGRES(res);
16501654
termStringInfo(&sql);
@@ -1654,25 +1658,27 @@ repack_one_index(Oid table, const char *table_name, Oid index, const char *schem
16541658
/*
16551659
* Call repack_one_index for each of the indexes
16561660
*/
1657-
static bool
1661+
static bool
16581662
repack_all_indexes(char *errbuf, size_t errsize){
16591663
bool ret = false;
1660-
PGresult *res = NULL, *res2 = NULL;
1661-
int i;
1662-
int num;
1663-
StringInfoData sql;
1664+
PGresult *res = NULL, *res2 = NULL;
1665+
int i;
1666+
int num;
1667+
StringInfoData sql;
16641668
const char *params[1];
16651669
const char *table_name = NULL;
16661670
const char *schema_name = NULL;
1667-
char *pos;
1671+
char *pos;
16681672

16691673
initStringInfo(&sql);
16701674
reconnect(ERROR);
16711675

16721676
if (!preliminary_checks(errbuf, errsize))
1673-
goto cleanup;
1677+
goto cleanup;
16741678

1675-
/* If only one index is specified, append the appropriate data to the sql and check if the index exists */
1679+
/* If only one index is specified, append the appropriate data to the sql
1680+
* and check if the index exists
1681+
*/
16761682
if (r_index)
16771683
{
16781684
appendStringInfoString(&sql, "SELECT i.relname, idx.indexrelid, idx.indisvalid, tbl.oid, tbl.relname"
@@ -1699,7 +1705,7 @@ repack_all_indexes(char *errbuf, size_t errsize){
16991705
goto cleanup;
17001706
}
17011707
}
1702-
1708+
17031709
// seperate schema name and index name
17041710
pos = strchr(params[0], '.');
17051711
if (pos)
@@ -1709,11 +1715,11 @@ repack_all_indexes(char *errbuf, size_t errsize){
17091715
r_index = pos + 1;
17101716
}
17111717
table_name = getstr(res, 0, 4);
1712-
}
1718+
}
17131719
/* To repack all indexes, append appropriate data to the sql and run the query */
17141720
else {
17151721
params[0] = table_list.head->val;
1716-
1722+
17171723
appendStringInfoString(&sql, "SELECT i.relname, idx.indexrelid, idx.indisvalid, idx.indrelid"
17181724
" FROM pg_index idx JOIN pg_class i ON i.oid = idx.indexrelid"
17191725
" WHERE idx.indrelid = $1::regclass");
@@ -1725,12 +1731,12 @@ repack_all_indexes(char *errbuf, size_t errsize){
17251731
snprintf(errbuf, errsize, "%s", PQerrorMessage(connection));
17261732
goto cleanup;
17271733
}
1728-
else
1734+
else
17291735
{
17301736
num = PQntuples(res);
17311737
if (num == 0)
17321738
{
1733-
elog(WARNING, "\"%s\" doesnot have any indexes", table_list.head->val);
1739+
elog(WARNING, "\"%s\" does not have any indexes", table_list.head->val);
17341740
ret = true;
17351741
goto cleanup;
17361742
}
@@ -1748,6 +1754,14 @@ repack_all_indexes(char *errbuf, size_t errsize){
17481754
table_name = params[0];
17491755
}
17501756

1757+
/* XXX: Make sure that repack_one_table() also obtains an advisory
1758+
* lock on the table, so that we can't have a table-wide repack running
1759+
* along with an indexes-only repack. Also, since advisory locks are
1760+
* 8 bytes wide and OIDs are only 4 bytes, consider using our own prefix
1761+
* rather than just the table OID, to avoid inadvertent conflict with
1762+
* other applications using advisory locks.
1763+
*/
1764+
17511765
/* Check if any concurrent pg_repack command is being run on the same table */
17521766
initStringInfo(&sql);
17531767
appendStringInfo(&sql, "SELECT pg_try_advisory_lock(%u)", getoid(res, 0, 3));
@@ -1767,7 +1781,7 @@ repack_all_indexes(char *errbuf, size_t errsize){
17671781
for (i = 0; i < num; i++)
17681782
{
17691783
char *isvalid = getstr(res, i, 2);
1770-
if (isvalid[0] == 't')
1784+
if (isvalid[0] == 't')
17711785
{
17721786
if (schema_name)
17731787
elog(INFO, "repacking index \"%s.%s\"", schema_name, getstr(res, i, 0));
@@ -1779,9 +1793,9 @@ repack_all_indexes(char *errbuf, size_t errsize){
17791793
}
17801794
else
17811795
if (schema_name)
1782-
elog(WARNING, "skipping invalid index: %s.%s", schema_name, getstr(res, i, 0));
1796+
elog(WARNING, "skipping invalid index: %s.%s", schema_name, getstr(res, i, 0));
17831797
else
1784-
elog(WARNING, "skipping invalid index: %s", getstr(res, i, 0));
1798+
elog(WARNING, "skipping invalid index: %s", getstr(res, i, 0));
17851799
}
17861800
ret = true;
17871801
cleanup:

0 commit comments

Comments
 (0)