Skip to content

Commit f06bec3

Browse files
andinuxandinux
committed
feat: add a table sanity check to discourage the use of INTEGER primary key to avoid conflicts across nodes.
If you understand the risk and still want to use this INTEGER primary key, set the third argument of the cloudsync_init function to 1 to skip this check.
1 parent 94d35e9 commit f06bec3

File tree

4 files changed

+94
-59
lines changed

4 files changed

+94
-59
lines changed

‎src/cloudsync.c‎

Lines changed: 52 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ typedef struct {
104104
int index;
105105
} cloudsync_pk_decode_context;
106106

107+
typedef struct {
108+
sqlite3_context *context;
109+
bool skip_int_pk_check;
110+
} cloudsync_init_all_context;
111+
107112
#define SYNCBIT_SET(_data) _data->insync = 1
108113
#define SYNCBIT_RESET(_data) _data->insync = 0
109114
#define BUMP_SEQ(_data) ((_data)->seq += 1, (_data)->seq - 1)
@@ -1502,8 +1507,11 @@ void cloudsync_context_free (void *ptr) {
15021507
const char *cloudsync_context_init (sqlite3 *db, cloudsync_context *data, sqlite3_context *context) {
15031508
if (!data && context) data = (cloudsync_context *)sqlite3_user_data(context);
15041509

1505-
// perform init just the first time, if the site_id field is not set
1506-
if (data->site_id[0] == 0) {
1510+
// perform init just the first time, if the site_id field is not set.
1511+
// The data->site_id value could exists while settings tables don't exists if the
1512+
// cloudsync_context_init was previously called in init transaction that was rolled back
1513+
// because of an error during the init process.
1514+
if (data->site_id[0] == 0 || !dbutils_table_exists(db, CLOUDSYNC_SITEID_NAME)) {
15071515
if (dbutils_settings_init(db, data, context) != SQLITE_OK) return NULL;
15081516
if (stmts_add_tocontext(db, data) != SQLITE_OK) return NULL;
15091517
if (cloudsync_load_siteid(db, data) != SQLITE_OK) return NULL;
@@ -2836,7 +2844,7 @@ int cloudsync_load_siteid (sqlite3 *db, cloudsync_context *data) {
28362844
return SQLITE_OK;
28372845
}
28382846

2839-
int cloudsync_init_internal (sqlite3_context *context, const char *table_name, const char *algo_name) {
2847+
int cloudsync_init_internal (sqlite3_context *context, const char *table_name, const char *algo_name, bool skip_int_pk_check) {
28402848
DEBUG_FUNCTION("cloudsync_init_internal");
28412849

28422850
// get database reference
@@ -2846,7 +2854,7 @@ int cloudsync_init_internal (sqlite3_context *context, const char *table_name, c
28462854
cloudsync_context *data = (cloudsync_context *)sqlite3_user_data(context);
28472855

28482856
// sanity check table and its primary key(s)
2849-
if (dbutils_table_sanity_check(db, context, table_name) == false) {
2857+
if (dbutils_table_sanity_check(db, context, table_name, skip_int_pk_check) == false) {
28502858
return SQLITE_MISUSE;
28512859
}
28522860

@@ -2920,13 +2928,15 @@ int cloudsync_init_internal (sqlite3_context *context, const char *table_name, c
29202928
}
29212929

29222930
int cloudsync_init_all_callback (void *xdata, int ncols, char **values, char **names) {
2923-
sqlite3_context *context = (sqlite3_context *)xdata;
2931+
cloudsync_init_all_context *init_ctx = (cloudsync_init_all_context *)xdata;
2932+
sqlite3_context *context = init_ctx->context;
2933+
bool skip_int_pk_check = init_ctx->skip_int_pk_check;
29242934

29252935
for (int i=0; i<ncols; i+=2) {
29262936
const char *table = values[i];
29272937
const char *algo = values[i+1];
29282938

2929-
int rc = cloudsync_init_internal(context, table, algo);
2939+
int rc = cloudsync_init_internal(context, table, algo, skip_int_pk_check);
29302940
if (rc != SQLITE_OK) {
29312941
cloudsync_cleanup_internal(context, table);
29322942
return rc;
@@ -2936,37 +2946,33 @@ int cloudsync_init_all_callback (void *xdata, int ncols, char **values, char **n
29362946
return SQLITE_OK;
29372947
}
29382948

2939-
int cloudsync_init_all (sqlite3_context *context, const char *algo_name) {
2949+
int cloudsync_init_all (sqlite3_context *context, const char *algo_name, bool skip_int_pk_check) {
29402950
char sql[1024];
29412951
snprintf(sql, sizeof(sql), "SELECT name, '%s' FROM sqlite_master WHERE type='table' and name NOT LIKE 'sqlite_%%' AND name NOT LIKE 'cloudsync_%%' AND name NOT LIKE '%%_cloudsync';", (algo_name) ? algo_name : CLOUDSYNC_DEFAULT_ALGO);
29422952

29432953
sqlite3 *db = sqlite3_context_db_handle(context);
2944-
int rc = sqlite3_exec(db, sql, cloudsync_init_all_callback, context, NULL);
2954+
cloudsync_init_all_context init_ctx = {.context = context, .skip_int_pk_check = skip_int_pk_check};
2955+
int rc = sqlite3_exec(db, sql, cloudsync_init_all_callback, &init_ctx, NULL);
29452956
return rc;
29462957
}
29472958

2948-
void cloudsync_init_algo (sqlite3_context *context, int argc, sqlite3_value **argv) {
2949-
DEBUG_FUNCTION("cloudsync_init_algo");
2950-
2951-
const char *table = (const char *)sqlite3_value_text(argv[0]);
2952-
const char *algo = (const char *)sqlite3_value_text(argv[1]);
2953-
2959+
void cloudsync_init (sqlite3_context *context, const char *table, const char *algo, bool skip_int_pk_check) {
29542960
cloudsync_context *data = (cloudsync_context *)sqlite3_user_data(context);
29552961
sqlite3 *db = sqlite3_context_db_handle(context);
2956-
int rc = sqlite3_exec(db, "SAVEPOINT cloudsync_init_algo;", NULL, NULL, NULL);
2962+
int rc = sqlite3_exec(db, "SAVEPOINT cloudsync_init;", NULL, NULL, NULL);
29572963
if (rc != SQLITE_OK) {
2958-
dbutils_context_result_error(context, "Unable to create cloudsync_init_algo savepoint. %s", sqlite3_errmsg(db));
2964+
dbutils_context_result_error(context, "Unable to create cloudsync_init savepoint. %s", sqlite3_errmsg(db));
29592965
sqlite3_result_error_code(context, rc);
29602966
return;
29612967
}
29622968

2963-
if (dbutils_is_star_table(table)) cloudsync_init_all(context, algo);
2964-
else cloudsync_init_internal(context, table, algo);
2969+
if (dbutils_is_star_table(table)) rc = cloudsync_init_all(context, algo, skip_int_pk_check);
2970+
else rc = cloudsync_init_internal(context, table, algo, skip_int_pk_check);
29652971

29662972
if (rc == SQLITE_OK) {
2967-
rc = sqlite3_exec(db, "RELEASE cloudsync_init_algo", NULL, NULL, NULL);
2973+
rc = sqlite3_exec(db, "RELEASE cloudsync_init", NULL, NULL, NULL);
29682974
if (rc != SQLITE_OK) {
2969-
dbutils_context_result_error(context, "Unable to release cloudsync_init_algo savepoint. %s", sqlite3_errmsg(db));
2975+
dbutils_context_result_error(context, "Unable to release cloudsync_init savepoint. %s", sqlite3_errmsg(db));
29702976
sqlite3_result_error_code(context, rc);
29712977
}
29722978
}
@@ -2975,33 +2981,31 @@ void cloudsync_init_algo (sqlite3_context *context, int argc, sqlite3_value **ar
29752981
else sqlite3_exec(db, "ROLLBACK", NULL, NULL, NULL);
29762982
}
29772983

2978-
void cloudsync_init (sqlite3_context *context, int argc, sqlite3_value **argv) {
2979-
DEBUG_FUNCTION("cloudsync_init");
2984+
void cloudsync_init3 (sqlite3_context *context, int argc, sqlite3_value **argv) {
2985+
DEBUG_FUNCTION("cloudsync_init2");
29802986

29812987
const char *table = (const char *)sqlite3_value_text(argv[0]);
2988+
const char *algo = (const char *)sqlite3_value_text(argv[1]);
2989+
bool skip_int_pk_check = (bool)sqlite3_value_int(argv[2]);
2990+
2991+
cloudsync_init(context, table, algo, skip_int_pk_check);
2992+
}
2993+
2994+
void cloudsync_init2 (sqlite3_context *context, int argc, sqlite3_value **argv) {
2995+
DEBUG_FUNCTION("cloudsync_init2");
29822996

2983-
cloudsync_context *data = (cloudsync_context *)sqlite3_user_data(context);
2984-
sqlite3 *db = sqlite3_context_db_handle(context);
2985-
int rc = sqlite3_exec(db, "SAVEPOINT cloudsync_init;", NULL, NULL, NULL);
2986-
if (rc != SQLITE_OK) {
2987-
dbutils_context_result_error(context, "Unable to create cloudsync_init savepoint. %s", sqlite3_errmsg(db));
2988-
sqlite3_result_error_code(context, rc);
2989-
return;
2990-
}
2997+
const char *table = (const char *)sqlite3_value_text(argv[0]);
2998+
const char *algo = (const char *)sqlite3_value_text(argv[1]);
2999+
3000+
cloudsync_init(context, table, algo, false);
3001+
}
29913002

2992-
if (dbutils_is_star_table(table)) rc = cloudsync_init_all(context, NULL);
2993-
else rc = cloudsync_init_internal(context, table, NULL);
3003+
void cloudsync_init1 (sqlite3_context *context, int argc, sqlite3_value **argv) {
3004+
DEBUG_FUNCTION("cloudsync_init1");
29943005

2995-
if (rc == SQLITE_OK) {
2996-
rc = sqlite3_exec(db, "RELEASE cloudsync_init", NULL, NULL, NULL);
2997-
if (rc != SQLITE_OK) {
2998-
dbutils_context_result_error(context, "Unable to release cloudsync_init savepoint. %s", sqlite3_errmsg(db));
2999-
sqlite3_result_error_code(context, rc);
3000-
}
3001-
}
3006+
const char *table = (const char *)sqlite3_value_text(argv[0]);
30023007

3003-
if (rc == SQLITE_OK) dbutils_update_schema_hash(db, &data->schema_hash);
3004-
else sqlite3_exec(db, "ROLLBACK", NULL, NULL, NULL);
3008+
cloudsync_init(context, table, NULL, false);
30053009
}
30063010

30073011
// MARK: -
@@ -3108,7 +3112,7 @@ void cloudsync_commit_alter (sqlite3_context *context, int argc, sqlite3_value *
31083112
// init again cloudsync for the table
31093113
table_algo algo_current = dbutils_table_settings_get_algo(db, table_name);
31103114
if (algo_current == table_algo_none) algo_current = dbutils_table_settings_get_algo(db, "*");
3111-
rc = cloudsync_init_internal(context, table_name, crdt_algo_name(algo_current));
3115+
rc = cloudsync_init_internal(context, table_name, crdt_algo_name(algo_current), true);
31123116
if (rc != SQLITE_OK) goto rollback_finalize_alter;
31133117

31143118
// release savepoint
@@ -3162,12 +3166,16 @@ APIEXPORT int sqlite3_cloudsync_init (sqlite3 *db, char **pzErrMsg, const sqlite
31623166
rc = dbutils_register_function(db, "cloudsync_version", cloudsync_version, 0, pzErrMsg, ctx, cloudsync_context_free);
31633167
if (rc != SQLITE_OK) return rc;
31643168

3165-
rc = dbutils_register_function(db, "cloudsync_init", cloudsync_init, 1, pzErrMsg, ctx, NULL);
3169+
rc = dbutils_register_function(db, "cloudsync_init", cloudsync_init1, 1, pzErrMsg, ctx, NULL);
31663170
if (rc != SQLITE_OK) return rc;
31673171

3168-
rc = dbutils_register_function(db, "cloudsync_init", cloudsync_init_algo, 2, pzErrMsg, ctx, NULL);
3172+
rc = dbutils_register_function(db, "cloudsync_init", cloudsync_init2, 2, pzErrMsg, ctx, NULL);
31693173
if (rc != SQLITE_OK) return rc;
31703174

3175+
rc = dbutils_register_function(db, "cloudsync_init", cloudsync_init3, 3, pzErrMsg, ctx, NULL);
3176+
if (rc != SQLITE_OK) return rc;
3177+
3178+
31713179
rc = dbutils_register_function(db, "cloudsync_enable", cloudsync_enable, 1, pzErrMsg, ctx, NULL);
31723180
if (rc != SQLITE_OK) return rc;
31733181

‎src/dbutils.c‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ bool dbutils_trigger_exists (sqlite3 *db, const char *name) {
364364
return dbutils_system_exists(db, name, "trigger");
365365
}
366366

367-
bool dbutils_table_sanity_check (sqlite3 *db, sqlite3_context *context, const char *name) {
367+
bool dbutils_table_sanity_check (sqlite3 *db, sqlite3_context *context, const char *name, bool skip_int_pk_check) {
368368
DEBUG_DBFUNCTION("dbutils_table_sanity_check %s", name);
369369

370370
char buffer[2048];
@@ -405,7 +405,21 @@ bool dbutils_table_sanity_check (sqlite3 *db, sqlite3_context *context, const ch
405405
return false;
406406
}
407407
#endif
408-
408+
409+
if (!skip_int_pk_check) {
410+
if (count == 1) {
411+
// the affinity of a column is determined by the declared type of the column,
412+
// according to the following rules in the order shown:
413+
// 1. If the declared type contains the string "INT" then it is assigned INTEGER affinity.
414+
sql = sqlite3_snprintf((int)blen, buffer, "SELECT count(*) FROM pragma_table_info('%w') WHERE pk=1 AND \"type\" LIKE \"%%INT%%\";", name);
415+
sqlite3_int64 count2 = dbutils_int_select(db, sql);
416+
if (count == count2) {
417+
dbutils_context_result_error(context, "Table %s uses an single-column INTEGER primary key. For CRDT replication, primary keys must be globally unique. Consider using a TEXT primary key with UUIDs or ULID to avoid conflicts across nodes. If you understand the risk and still want to use this INTEGER primary key, set the third argument of the cloudsync_init function to 1 to skip this check.", name);
418+
return false;
419+
}
420+
}
421+
}
422+
409423
// if user declared explicit primary key(s) then make sure they are all declared as NOT NULL
410424
if (count > 0) {
411425
sql = sqlite3_snprintf((int)blen, buffer, "SELECT count(*) FROM pragma_table_info('%w') WHERE pk>0 AND \"notnull\"=1;", name);
@@ -798,7 +812,9 @@ char *dbutils_table_settings_get_value (sqlite3 *db, const char *table, const ch
798812
#if CLOUDSYNC_UNITTEST
799813
if ((rc == SQLITE_NOMEM) && (size == SQLITE_MAX_ALLOCATION_SIZE + 1)) rc = SQLITE_OK;
800814
#endif
801-
if (rc != SQLITE_OK) DEBUG_ALWAYS("cloudsync_table_settings error %s", sqlite3_errmsg(db));
815+
if (rc != SQLITE_OK) {
816+
DEBUG_ALWAYS("cloudsync_table_settings error %s", sqlite3_errmsg(db));
817+
}
802818
if (vm) sqlite3_finalize(vm);
803819

804820
return buffer;

‎src/dbutils.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ void dbutils_context_result_error (sqlite3_context *context, const char *format,
4747
bool dbutils_system_exists (sqlite3 *db, const char *name, const char *type);
4848
bool dbutils_table_exists (sqlite3 *db, const char *name);
4949
bool dbutils_trigger_exists (sqlite3 *db, const char *name);
50-
bool dbutils_table_sanity_check (sqlite3 *db, sqlite3_context *context, const char *name);
50+
bool dbutils_table_sanity_check (sqlite3 *db, sqlite3_context *context, const char *name, bool skip_int_pk_check);
5151
bool dbutils_is_star_table (const char *table_name);
5252

5353
int dbutils_delete_triggers (sqlite3 *db, const char *table);

‎test/unit.c‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,7 +1690,10 @@ bool do_test_dbutils (void) {
16901690
"CREATE TABLE IF NOT EXISTS rowid_table (name TEXT, age INTEGER);"
16911691
"CREATE TABLE IF NOT EXISTS nonnull_prikey_table (name TEXT PRIMARY KEY, age INTEGER);"
16921692
"CREATE TABLE IF NOT EXISTS nonnull_nodefault_table (name TEXT PRIMARY KEY NOT NULL, stamp TEXT NOT NULL);"
1693-
"CREATE TABLE IF NOT EXISTS nonnull_default_table (name TEXT PRIMARY KEY NOT NULL, stamp TEXT NOT NULL DEFAULT CURRENT_TIME);";
1693+
"CREATE TABLE IF NOT EXISTS nonnull_default_table (name TEXT PRIMARY KEY NOT NULL, stamp TEXT NOT NULL DEFAULT CURRENT_TIME);"
1694+
"CREATE TABLE IF NOT EXISTS integer_pk (id INTEGER PRIMARY KEY NOT NULL, value);"
1695+
"CREATE TABLE IF NOT EXISTS int_pk (id INT PRIMARY KEY NOT NULL, value);";
1696+
;
16941697

16951698
rc = sqlite3_exec(db, sql, NULL, NULL, NULL);
16961699
if (rc != SQLITE_OK) goto finalize;
@@ -1737,27 +1740,35 @@ bool do_test_dbutils (void) {
17371740
if (b == true) goto finalize;
17381741

17391742
// test dbutils_table_sanity_check
1740-
b = dbutils_table_sanity_check(db, NULL, NULL);
1743+
b = dbutils_table_sanity_check(db, NULL, NULL, false);
17411744
if (b == true) goto finalize;
1742-
b = dbutils_table_sanity_check(db, NULL, "rowid_table");
1745+
b = dbutils_table_sanity_check(db, NULL, "rowid_table", false);
17431746
if (b == true) goto finalize;
1744-
b = dbutils_table_sanity_check(db, NULL, "foo2");
1747+
b = dbutils_table_sanity_check(db, NULL, "foo2", false);
17451748
if (b == true) goto finalize;
1746-
b = dbutils_table_sanity_check(db, NULL, build_long_tablename());
1749+
b = dbutils_table_sanity_check(db, NULL, build_long_tablename(), false);
17471750
if (b == true) goto finalize;
1748-
b = dbutils_table_sanity_check(db, NULL, "nonnull_prikey_table");
1751+
b = dbutils_table_sanity_check(db, NULL, "nonnull_prikey_table", false);
17491752
if (b == true) goto finalize;
1750-
b = dbutils_table_sanity_check(db, NULL, "nonnull_nodefault_table");
1753+
b = dbutils_table_sanity_check(db, NULL, "nonnull_nodefault_table", false);
17511754
if (b == true) goto finalize;
1752-
b = dbutils_table_sanity_check(db, NULL, "nonnull_default_table");
1755+
b = dbutils_table_sanity_check(db, NULL, "nonnull_default_table", false);
1756+
if (b == false) goto finalize;
1757+
b = dbutils_table_sanity_check(db, NULL, "integer_pk", false);
1758+
if (b == true) goto finalize;
1759+
b = dbutils_table_sanity_check(db, NULL, "integer_pk", true);
1760+
if (b == false) goto finalize;
1761+
b = dbutils_table_sanity_check(db, NULL, "int_pk", false);
1762+
if (b == true) goto finalize;
1763+
b = dbutils_table_sanity_check(db, NULL, "int_pk", true);
17531764
if (b == false) goto finalize;
17541765

17551766
// create huge dummy_table table
17561767
rc = sqlite3_exec(db, build_huge_table(), NULL, NULL, NULL);
17571768
if (rc != SQLITE_OK) goto finalize;
17581769

17591770
// sanity check the huge dummy_table table
1760-
b = dbutils_table_sanity_check(db, NULL, "dummy_table");
1771+
b = dbutils_table_sanity_check(db, NULL, "dummy_table", false);
17611772
if (b == true) goto finalize;
17621773

17631774
// de-augment bar with cloudsync
@@ -2789,7 +2800,7 @@ bool do_test_prikey (int nclients, bool print_result, bool cleanup_databases) {
27892800
rc = sqlite3_exec(db[i], sql, NULL, NULL, NULL);
27902801
if (rc != SQLITE_OK) goto finalize;
27912802

2792-
sql = "SELECT cloudsync_init('foo');";
2803+
sql = "SELECT cloudsync_init('foo', 'cls', 1);";
27932804
rc = sqlite3_exec(db[i], sql, NULL, NULL, NULL);
27942805
if (rc != SQLITE_OK) goto finalize;
27952806
}
@@ -2920,7 +2931,7 @@ bool do_test_gos (int nclients, bool print_result, bool cleanup_databases) {
29202931
rc = sqlite3_exec(db[i], sql, NULL, NULL, NULL);
29212932
if (rc != SQLITE_OK) goto finalize;
29222933

2923-
sql = "SELECT cloudsync_init('log', 'gos');";
2934+
sql = "SELECT cloudsync_init('log', 'gos', 0);";
29242935
rc = sqlite3_exec(db[i], sql, NULL, NULL, NULL);
29252936
if (rc != SQLITE_OK) goto finalize;
29262937
}

0 commit comments

Comments
 (0)