Skip to content

Commit 06338dc

Browse files
committed
fix: make begin_alter and commit_alter idempotent
Add is_altering flag to cloudsync_table_context to prevent errors when alter functions are called multiple times on the same table. begin_alter returns early if already altering; commit_alter returns early if not.
1 parent c25bd24 commit 06338dc

1 file changed

Lines changed: 26 additions & 12 deletions

File tree

src/cloudsync.c

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,8 @@ struct cloudsync_table_context {
224224
dbvm_t *real_merge_delete_stmt;
225225
dbvm_t *real_merge_sentinel_stmt;
226226

227+
bool is_altering; // flag to track if a table alteration is in progress
228+
227229
// context
228230
cloudsync_context *context;
229231
};
@@ -2264,15 +2266,18 @@ int cloudsync_begin_alter (cloudsync_context *data, const char *table_name) {
22642266
if (cloudsync_context_init(data) == NULL) {
22652267
return DBRES_MISUSE;
22662268
}
2267-
2269+
22682270
// lookup table
22692271
cloudsync_table_context *table = table_lookup(data, table_name);
22702272
if (!table) {
22712273
char buffer[1024];
22722274
snprintf(buffer, sizeof(buffer), "Unable to find table %s", table_name);
22732275
return cloudsync_set_error(data, buffer, DBRES_MISUSE);
22742276
}
2275-
2277+
2278+
// idempotent: if already altering, return OK
2279+
if (table->is_altering) return DBRES_OK;
2280+
22762281
// retrieve primary key(s)
22772282
char **names = NULL;
22782283
int nrows = 0;
@@ -2283,15 +2288,15 @@ int cloudsync_begin_alter (cloudsync_context *data, const char *table_name) {
22832288
cloudsync_set_error(data, buffer, DBRES_MISUSE);
22842289
goto rollback_begin_alter;
22852290
}
2286-
2291+
22872292
// sanity check the number of primary keys
22882293
if (nrows != table_count_pks(table)) {
22892294
char buffer[1024];
22902295
snprintf(buffer, sizeof(buffer), "Number of primary keys for table %s changed before ALTER", table_name);
22912296
cloudsync_set_error(data, buffer, DBRES_MISUSE);
22922297
goto rollback_begin_alter;
22932298
}
2294-
2299+
22952300
// drop original triggers
22962301
rc = database_delete_triggers(data, table_name);
22972302
if (rc != DBRES_OK) {
@@ -2300,10 +2305,11 @@ int cloudsync_begin_alter (cloudsync_context *data, const char *table_name) {
23002305
cloudsync_set_error(data, buffer, DBRES_ERROR);
23012306
goto rollback_begin_alter;
23022307
}
2303-
2308+
23042309
table_set_pknames(table, names);
2310+
table->is_altering = true;
23052311
return DBRES_OK;
2306-
2312+
23072313
rollback_begin_alter:
23082314
if (names) table_pknames_free(names, nrows);
23092315
return rc;
@@ -2393,13 +2399,13 @@ int cloudsync_finalize_alter (cloudsync_context *data, cloudsync_table_context *
23932399
int cloudsync_commit_alter (cloudsync_context *data, const char *table_name) {
23942400
int rc = DBRES_MISUSE;
23952401
cloudsync_table_context *table = NULL;
2396-
2402+
23972403
// init cloudsync_settings
23982404
if (cloudsync_context_init(data) == NULL) {
23992405
cloudsync_set_error(data, "Unable to initialize cloudsync context", DBRES_MISUSE);
24002406
goto rollback_finalize_alter;
24012407
}
2402-
2408+
24032409
// lookup table
24042410
table = table_lookup(data, table_name);
24052411
if (!table) {
@@ -2408,15 +2414,20 @@ int cloudsync_commit_alter (cloudsync_context *data, const char *table_name) {
24082414
cloudsync_set_error(data, buffer, DBRES_MISUSE);
24092415
goto rollback_finalize_alter;
24102416
}
2411-
2417+
2418+
// idempotent: if not altering, return OK
2419+
if (!table->is_altering) return DBRES_OK;
2420+
24122421
rc = cloudsync_finalize_alter(data, table);
24132422
if (rc != DBRES_OK) goto rollback_finalize_alter;
2414-
2423+
24152424
// the table is outdated, delete it and it will be reloaded in the cloudsync_init_internal
2425+
// is_altering is reset implicitly because table_free + cloudsync_init_table
2426+
// will reallocate the table context with zero-initialized memory
24162427
table_remove(data, table);
24172428
table_free(table);
24182429
table = NULL;
2419-
2430+
24202431
// init again cloudsync for the table
24212432
table_algo algo_current = dbutils_table_settings_get_algo(data, table_name);
24222433
if (algo_current == table_algo_none) algo_current = dbutils_table_settings_get_algo(data, "*");
@@ -2426,7 +2437,10 @@ int cloudsync_commit_alter (cloudsync_context *data, const char *table_name) {
24262437
return DBRES_OK;
24272438

24282439
rollback_finalize_alter:
2429-
if (table) table_set_pknames(table, NULL);
2440+
if (table) {
2441+
table_set_pknames(table, NULL);
2442+
table->is_altering = false;
2443+
}
24302444
return rc;
24312445
}
24322446

0 commit comments

Comments
 (0)