From 9b6e28f5e1f18bc1ccb219f4ac552f14d2990e55 Mon Sep 17 00:00:00 2001 From: Marco Bambini Date: Mon, 9 Mar 2026 08:42:34 +0100 Subject: [PATCH 1/5] Added a new compilation macro CLOUDSYNC_CHECK_NOTNULL_PRIKEYS (disabled by default) --- src/cloudsync.c | 2 ++ src/pk.c | 11 ++++++++++- src/pk.h | 2 ++ src/postgresql/cloudsync_postgresql.c | 14 +++++++++++++- src/sqlite/cloudsync_sqlite.c | 17 ++++++++++++++++- test/unit.c | 27 ++++++--------------------- 6 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/cloudsync.c b/src/cloudsync.c index 12c0e90..170725e 100644 --- a/src/cloudsync.c +++ b/src/cloudsync.c @@ -2678,6 +2678,7 @@ int cloudsync_table_sanity_check (cloudsync_context *data, const char *name, boo } // if user declared explicit primary key(s) then make sure they are all declared as NOT NULL + #if CLOUDSYNC_CHECK_NOTNULL_PRIKEYS if (npri_keys > 0) { int npri_keys_notnull = database_count_pk(data, name, true, cloudsync_schema(data)); if (npri_keys_notnull < 0) return cloudsync_set_dberror(data); @@ -2686,6 +2687,7 @@ int cloudsync_table_sanity_check (cloudsync_context *data, const char *name, boo return cloudsync_set_error(data, buffer, DBRES_ERROR); } } + #endif // check for columns declared as NOT NULL without a DEFAULT value. // Otherwise, col_merge_stmt would fail if changes to other columns are inserted first. diff --git a/src/pk.c b/src/pk.c index cd7899b..97a6639 100644 --- a/src/pk.c +++ b/src/pk.c @@ -87,6 +87,8 @@ #define DATABASE_TYPE_MAX_NEGATIVE_INTEGER 6 // was SQLITE_MAX_NEGATIVE_INTEGER #define DATABASE_TYPE_NEGATIVE_FLOAT 7 // was SQLITE_NEGATIVE_FLOAT +char * const PRIKEY_NULL_CONSTRAINT_ERROR = "PRIKEY_NULL_CONSTRAINT_ERROR"; + // MARK: - Public Callbacks - int pk_decode_bind_callback (void *xdata, int index, int type, int64_t ival, double dval, char *pval) { @@ -436,7 +438,14 @@ char *pk_encode (dbvalue_t **argv, int argc, char *b, bool is_prikey, size_t *bs if (!bsize) return NULL; // must fit in a single byte if (argc > 255) return NULL; - + + // if schema does not enforce NOT NULL on primary keys, check at runtime + #ifndef CLOUDSYNC_CHECK_NOTNULL_PRIKEYS + for (int i = 0; i < argc; i++) { + if (database_value_type(argv[i]) == DBTYPE_NULL) return PRIKEY_NULL_CONSTRAINT_ERROR; + } + #endif + // 1 is the number of items in the serialization // always 1 byte so max 255 primary keys, even if there is an hard SQLite limit of 128 size_t blen_curr = *bsize; diff --git a/src/pk.h b/src/pk.h index 2571915..ea9a390 100644 --- a/src/pk.h +++ b/src/pk.h @@ -15,6 +15,8 @@ typedef int (*pk_decode_callback) (void *xdata, int index, int type, int64_t ival, double dval, char *pval); +extern char * const PRIKEY_NULL_CONSTRAINT_ERROR; + char *pk_encode_prikey (dbvalue_t **argv, int argc, char *b, size_t *bsize); char *pk_encode_value (dbvalue_t *value, size_t *bsize); char *pk_encode (dbvalue_t **argv, int argc, char *b, bool is_prikey, size_t *bsize, int skip_idx); diff --git a/src/postgresql/cloudsync_postgresql.c b/src/postgresql/cloudsync_postgresql.c index 09df63b..21308cf 100644 --- a/src/postgresql/cloudsync_postgresql.c +++ b/src/postgresql/cloudsync_postgresql.c @@ -1122,7 +1122,7 @@ Datum cloudsync_pk_encode (PG_FUNCTION_ARGS) { size_t pklen = 0; char *encoded = pk_encode_prikey((dbvalue_t **)argv, argc, NULL, &pklen); - if (!encoded) { + if (!encoded || encoded == PRIKEY_NULL_CONSTRAINT_ERROR) { ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), errmsg("cloudsync_pk_encode failed to encode primary key"))); } @@ -1271,6 +1271,10 @@ Datum cloudsync_insert (PG_FUNCTION_ARGS) { if (!cleanup.pk) { ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("Not enough memory to encode the primary key(s)"))); } + if (cleanup.pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + cleanup.pk = NULL; + ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("Insert aborted because primary key in table %s contains NULL values", table_name))); + } // Compute the next database version for tracking changes int64_t db_version = cloudsync_dbversion_next(data, CLOUDSYNC_VALUE_NOTSET); @@ -1360,6 +1364,10 @@ Datum cloudsync_delete (PG_FUNCTION_ARGS) { if (!cleanup.pk) { ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("Not enough memory to encode the primary key(s)"))); } + if (cleanup.pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + cleanup.pk = NULL; + ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("Delete aborted because primary key in table %s contains NULL values", table_name))); + } int64_t db_version = cloudsync_dbversion_next(data, CLOUDSYNC_VALUE_NOTSET); @@ -1561,6 +1569,10 @@ Datum cloudsync_update_finalfn (PG_FUNCTION_ARGS) { if (!pk) { ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("Not enough memory to encode the primary key(s)"))); } + if (pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + pk = NULL; + ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("Update aborted because primary key in table %s contains NULL values", table_name))); + } if (prikey_changed) { oldpk = pk_encode_prikey((dbvalue_t **)payload->old_values, pk_count, buffer2, &oldpklen); if (!oldpk) { diff --git a/src/sqlite/cloudsync_sqlite.c b/src/sqlite/cloudsync_sqlite.c index 08268b3..8333111 100644 --- a/src/sqlite/cloudsync_sqlite.c +++ b/src/sqlite/cloudsync_sqlite.c @@ -260,7 +260,7 @@ void dbsync_col_value (sqlite3_context *context, int argc, sqlite3_value **argv) void dbsync_pk_encode (sqlite3_context *context, int argc, sqlite3_value **argv) { size_t bsize = 0; char *buffer = pk_encode_prikey((dbvalue_t **)argv, argc, NULL, &bsize); - if (!buffer) { + if (!buffer || buffer == PRIKEY_NULL_CONSTRAINT_ERROR) { sqlite3_result_null(context); return; } @@ -347,6 +347,10 @@ void dbsync_insert (sqlite3_context *context, int argc, sqlite3_value **argv) { sqlite3_result_error(context, "Not enough memory to encode the primary key(s).", -1); return; } + if (pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + dbsync_set_error(context, "Insert aborted because primary key in table %s contains NULL values.", table_name); + return; + } // compute the next database version for tracking changes int64_t db_version = cloudsync_dbversion_next(data, CLOUDSYNC_VALUE_NOTSET); @@ -407,6 +411,11 @@ void dbsync_delete (sqlite3_context *context, int argc, sqlite3_value **argv) { return; } + if (pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + dbsync_set_error(context, "Delete aborted because primary key in table %s contains NULL values.", table_name); + return; + } + // mark the row as deleted by inserting a delete sentinel into the metadata rc = local_mark_delete_meta(table, pk, pklen, db_version, cloudsync_bumpseq(data)); if (rc != SQLITE_OK) goto cleanup; @@ -542,6 +551,11 @@ void dbsync_update_final (sqlite3_context *context) { dbsync_update_payload_free(payload); return; } + if (pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + dbsync_set_error(context, "Update aborted because primary key in table %s contains NULL values.", table_name); + dbsync_update_payload_free(payload); + return; + } if (prikey_changed) { // if the primary key has changed, we need to handle the row differently: @@ -551,6 +565,7 @@ void dbsync_update_final (sqlite3_context *context) { // encode the OLD primary key into a buffer oldpk = pk_encode_prikey((dbvalue_t **)payload->old_values, table_count_pks(table), buffer2, &oldpklen); if (!oldpk) { + // no check here about PRIKEY_NULL_CONSTRAINT_ERROR because by design oldpk cannot contain NULL values if (pk != buffer) cloudsync_memory_free(pk); sqlite3_result_error(context, "Not enough memory to encode the primary key(s).", -1); dbsync_update_payload_free(payload); diff --git a/test/unit.c b/test/unit.c index 80ac905..9f1df5c 100644 --- a/test/unit.c +++ b/test/unit.c @@ -1716,7 +1716,7 @@ bool do_test_pk (sqlite3 *db, int ntest, bool print_result) { if (do_test_pk_single_value(db, SQLITE_INTEGER, -15592946911031981, 0, NULL, print_result) == false) goto finalize; if (do_test_pk_single_value(db, SQLITE_INTEGER, -922337203685477580, 0, NULL, print_result) == false) goto finalize; if (do_test_pk_single_value(db, SQLITE_FLOAT, 0, -9223372036854775.808, NULL, print_result) == false) goto finalize; - if (do_test_pk_single_value(db, SQLITE_NULL, 0, 0, NULL, print_result) == false) goto finalize; + // SQLITE_NULL is no longer valid for primary keys (runtime NULL check rejects it) if (do_test_pk_single_value(db, SQLITE_TEXT, 0, 0, "Hello World", print_result) == false) goto finalize; char blob[] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16}; if (do_test_pk_single_value(db, SQLITE_BLOB, sizeof(blob), 0, blob, print_result) == false) goto finalize; @@ -2381,8 +2381,8 @@ bool do_test_pk_decode_count_from_buffer(void) { rc = sqlite3_cloudsync_init(db, NULL, NULL); if (rc != SQLITE_OK) goto cleanup; - // Encode multiple values - const char *sql = "SELECT cloudsync_pk_encode(123, 'text value', 3.14, X'DEADBEEF', NULL);"; + // Encode multiple values (no NULL — primary keys cannot contain NULL) + const char *sql = "SELECT cloudsync_pk_encode(123, 'text value', 3.14, X'DEADBEEF');"; rc = sqlite3_prepare_v2(db, sql, -1, &stmt, NULL); if (rc != SQLITE_OK) goto cleanup; @@ -2403,7 +2403,7 @@ bool do_test_pk_decode_count_from_buffer(void) { // The count is embedded in the first byte of the encoded pk size_t seek = 0; int n = pk_decode(buffer, (size_t)pklen, -1, &seek, -1, NULL, NULL); - if (n != 5) goto cleanup; // Should decode 5 values + if (n != 4) goto cleanup; // Should decode 4 values result = true; @@ -2849,8 +2849,8 @@ bool do_test_sql_pk_decode(void) { rc = sqlite3_cloudsync_init(db, NULL, NULL); if (rc != SQLITE_OK) goto cleanup; - // Create a primary key with multiple values - rc = sqlite3_prepare_v2(db, "SELECT cloudsync_pk_encode(123, 'hello', 3.14, X'DEADBEEF', NULL);", -1, &stmt, NULL); + // Create a primary key with multiple values (no NULL — primary keys cannot contain NULL) + rc = sqlite3_prepare_v2(db, "SELECT cloudsync_pk_encode(123, 'hello', 3.14, X'DEADBEEF');", -1, &stmt, NULL); if (rc != SQLITE_OK) goto cleanup; rc = sqlite3_step(stmt); @@ -2934,21 +2934,6 @@ bool do_test_sql_pk_decode(void) { sqlite3_finalize(stmt); stmt = NULL; - // Test cloudsync_pk_decode for NULL (index 5) - rc = sqlite3_prepare_v2(db, "SELECT cloudsync_pk_decode(?, 5);", -1, &stmt, NULL); - if (rc != SQLITE_OK) goto cleanup; - - rc = sqlite3_bind_blob(stmt, 1, pk_copy, pk_len, SQLITE_STATIC); - if (rc != SQLITE_OK) goto cleanup; - - rc = sqlite3_step(stmt); - if (rc != SQLITE_ROW) goto cleanup; - - if (sqlite3_column_type(stmt, 0) != SQLITE_NULL) goto cleanup; - - sqlite3_finalize(stmt); - stmt = NULL; - result = true; cleanup: From b3e507459ee5f0d97c529d9cfe57a460e2fbd64d Mon Sep 17 00:00:00 2001 From: Marco Bambini Date: Mon, 9 Mar 2026 08:42:34 +0100 Subject: [PATCH 2/5] Added a new compilation macro CLOUDSYNC_CHECK_NOTNULL_PRIKEYS (disabled by default) --- src/cloudsync.c | 2 ++ src/pk.c | 11 ++++++++++- src/pk.h | 2 ++ src/postgresql/cloudsync_postgresql.c | 14 +++++++++++++- src/sqlite/cloudsync_sqlite.c | 17 ++++++++++++++++- test/unit.c | 27 ++++++--------------------- 6 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/cloudsync.c b/src/cloudsync.c index 64e2ce6..e0c454e 100644 --- a/src/cloudsync.c +++ b/src/cloudsync.c @@ -2986,6 +2986,7 @@ int cloudsync_table_sanity_check (cloudsync_context *data, const char *name, boo } // if user declared explicit primary key(s) then make sure they are all declared as NOT NULL + #if CLOUDSYNC_CHECK_NOTNULL_PRIKEYS if (npri_keys > 0) { int npri_keys_notnull = database_count_pk(data, name, true, cloudsync_schema(data)); if (npri_keys_notnull < 0) return cloudsync_set_dberror(data); @@ -2994,6 +2995,7 @@ int cloudsync_table_sanity_check (cloudsync_context *data, const char *name, boo return cloudsync_set_error(data, buffer, DBRES_ERROR); } } + #endif // check for columns declared as NOT NULL without a DEFAULT value. // Otherwise, col_merge_stmt would fail if changes to other columns are inserted first. diff --git a/src/pk.c b/src/pk.c index cd7899b..97a6639 100644 --- a/src/pk.c +++ b/src/pk.c @@ -87,6 +87,8 @@ #define DATABASE_TYPE_MAX_NEGATIVE_INTEGER 6 // was SQLITE_MAX_NEGATIVE_INTEGER #define DATABASE_TYPE_NEGATIVE_FLOAT 7 // was SQLITE_NEGATIVE_FLOAT +char * const PRIKEY_NULL_CONSTRAINT_ERROR = "PRIKEY_NULL_CONSTRAINT_ERROR"; + // MARK: - Public Callbacks - int pk_decode_bind_callback (void *xdata, int index, int type, int64_t ival, double dval, char *pval) { @@ -436,7 +438,14 @@ char *pk_encode (dbvalue_t **argv, int argc, char *b, bool is_prikey, size_t *bs if (!bsize) return NULL; // must fit in a single byte if (argc > 255) return NULL; - + + // if schema does not enforce NOT NULL on primary keys, check at runtime + #ifndef CLOUDSYNC_CHECK_NOTNULL_PRIKEYS + for (int i = 0; i < argc; i++) { + if (database_value_type(argv[i]) == DBTYPE_NULL) return PRIKEY_NULL_CONSTRAINT_ERROR; + } + #endif + // 1 is the number of items in the serialization // always 1 byte so max 255 primary keys, even if there is an hard SQLite limit of 128 size_t blen_curr = *bsize; diff --git a/src/pk.h b/src/pk.h index 2571915..ea9a390 100644 --- a/src/pk.h +++ b/src/pk.h @@ -15,6 +15,8 @@ typedef int (*pk_decode_callback) (void *xdata, int index, int type, int64_t ival, double dval, char *pval); +extern char * const PRIKEY_NULL_CONSTRAINT_ERROR; + char *pk_encode_prikey (dbvalue_t **argv, int argc, char *b, size_t *bsize); char *pk_encode_value (dbvalue_t *value, size_t *bsize); char *pk_encode (dbvalue_t **argv, int argc, char *b, bool is_prikey, size_t *bsize, int skip_idx); diff --git a/src/postgresql/cloudsync_postgresql.c b/src/postgresql/cloudsync_postgresql.c index 09df63b..21308cf 100644 --- a/src/postgresql/cloudsync_postgresql.c +++ b/src/postgresql/cloudsync_postgresql.c @@ -1122,7 +1122,7 @@ Datum cloudsync_pk_encode (PG_FUNCTION_ARGS) { size_t pklen = 0; char *encoded = pk_encode_prikey((dbvalue_t **)argv, argc, NULL, &pklen); - if (!encoded) { + if (!encoded || encoded == PRIKEY_NULL_CONSTRAINT_ERROR) { ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), errmsg("cloudsync_pk_encode failed to encode primary key"))); } @@ -1271,6 +1271,10 @@ Datum cloudsync_insert (PG_FUNCTION_ARGS) { if (!cleanup.pk) { ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("Not enough memory to encode the primary key(s)"))); } + if (cleanup.pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + cleanup.pk = NULL; + ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("Insert aborted because primary key in table %s contains NULL values", table_name))); + } // Compute the next database version for tracking changes int64_t db_version = cloudsync_dbversion_next(data, CLOUDSYNC_VALUE_NOTSET); @@ -1360,6 +1364,10 @@ Datum cloudsync_delete (PG_FUNCTION_ARGS) { if (!cleanup.pk) { ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("Not enough memory to encode the primary key(s)"))); } + if (cleanup.pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + cleanup.pk = NULL; + ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("Delete aborted because primary key in table %s contains NULL values", table_name))); + } int64_t db_version = cloudsync_dbversion_next(data, CLOUDSYNC_VALUE_NOTSET); @@ -1561,6 +1569,10 @@ Datum cloudsync_update_finalfn (PG_FUNCTION_ARGS) { if (!pk) { ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("Not enough memory to encode the primary key(s)"))); } + if (pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + pk = NULL; + ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("Update aborted because primary key in table %s contains NULL values", table_name))); + } if (prikey_changed) { oldpk = pk_encode_prikey((dbvalue_t **)payload->old_values, pk_count, buffer2, &oldpklen); if (!oldpk) { diff --git a/src/sqlite/cloudsync_sqlite.c b/src/sqlite/cloudsync_sqlite.c index 08268b3..8333111 100644 --- a/src/sqlite/cloudsync_sqlite.c +++ b/src/sqlite/cloudsync_sqlite.c @@ -260,7 +260,7 @@ void dbsync_col_value (sqlite3_context *context, int argc, sqlite3_value **argv) void dbsync_pk_encode (sqlite3_context *context, int argc, sqlite3_value **argv) { size_t bsize = 0; char *buffer = pk_encode_prikey((dbvalue_t **)argv, argc, NULL, &bsize); - if (!buffer) { + if (!buffer || buffer == PRIKEY_NULL_CONSTRAINT_ERROR) { sqlite3_result_null(context); return; } @@ -347,6 +347,10 @@ void dbsync_insert (sqlite3_context *context, int argc, sqlite3_value **argv) { sqlite3_result_error(context, "Not enough memory to encode the primary key(s).", -1); return; } + if (pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + dbsync_set_error(context, "Insert aborted because primary key in table %s contains NULL values.", table_name); + return; + } // compute the next database version for tracking changes int64_t db_version = cloudsync_dbversion_next(data, CLOUDSYNC_VALUE_NOTSET); @@ -407,6 +411,11 @@ void dbsync_delete (sqlite3_context *context, int argc, sqlite3_value **argv) { return; } + if (pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + dbsync_set_error(context, "Delete aborted because primary key in table %s contains NULL values.", table_name); + return; + } + // mark the row as deleted by inserting a delete sentinel into the metadata rc = local_mark_delete_meta(table, pk, pklen, db_version, cloudsync_bumpseq(data)); if (rc != SQLITE_OK) goto cleanup; @@ -542,6 +551,11 @@ void dbsync_update_final (sqlite3_context *context) { dbsync_update_payload_free(payload); return; } + if (pk == PRIKEY_NULL_CONSTRAINT_ERROR) { + dbsync_set_error(context, "Update aborted because primary key in table %s contains NULL values.", table_name); + dbsync_update_payload_free(payload); + return; + } if (prikey_changed) { // if the primary key has changed, we need to handle the row differently: @@ -551,6 +565,7 @@ void dbsync_update_final (sqlite3_context *context) { // encode the OLD primary key into a buffer oldpk = pk_encode_prikey((dbvalue_t **)payload->old_values, table_count_pks(table), buffer2, &oldpklen); if (!oldpk) { + // no check here about PRIKEY_NULL_CONSTRAINT_ERROR because by design oldpk cannot contain NULL values if (pk != buffer) cloudsync_memory_free(pk); sqlite3_result_error(context, "Not enough memory to encode the primary key(s).", -1); dbsync_update_payload_free(payload); diff --git a/test/unit.c b/test/unit.c index 1caba38..1157ee5 100644 --- a/test/unit.c +++ b/test/unit.c @@ -1561,7 +1561,7 @@ bool do_test_pk (sqlite3 *db, int ntest, bool print_result) { if (do_test_pk_single_value(db, SQLITE_INTEGER, -15592946911031981, 0, NULL, print_result) == false) goto finalize; if (do_test_pk_single_value(db, SQLITE_INTEGER, -922337203685477580, 0, NULL, print_result) == false) goto finalize; if (do_test_pk_single_value(db, SQLITE_FLOAT, 0, -9223372036854775.808, NULL, print_result) == false) goto finalize; - if (do_test_pk_single_value(db, SQLITE_NULL, 0, 0, NULL, print_result) == false) goto finalize; + // SQLITE_NULL is no longer valid for primary keys (runtime NULL check rejects it) if (do_test_pk_single_value(db, SQLITE_TEXT, 0, 0, "Hello World", print_result) == false) goto finalize; char blob[] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16}; if (do_test_pk_single_value(db, SQLITE_BLOB, sizeof(blob), 0, blob, print_result) == false) goto finalize; @@ -2225,8 +2225,8 @@ bool do_test_pk_decode_count_from_buffer(void) { rc = sqlite3_cloudsync_init(db, NULL, NULL); if (rc != SQLITE_OK) goto cleanup; - // Encode multiple values - const char *sql = "SELECT cloudsync_pk_encode(123, 'text value', 3.14, X'DEADBEEF', NULL);"; + // Encode multiple values (no NULL — primary keys cannot contain NULL) + const char *sql = "SELECT cloudsync_pk_encode(123, 'text value', 3.14, X'DEADBEEF');"; rc = sqlite3_prepare_v2(db, sql, -1, &stmt, NULL); if (rc != SQLITE_OK) goto cleanup; @@ -2247,7 +2247,7 @@ bool do_test_pk_decode_count_from_buffer(void) { // The count is embedded in the first byte of the encoded pk size_t seek = 0; int n = pk_decode(buffer, (size_t)pklen, -1, &seek, -1, NULL, NULL); - if (n != 5) goto cleanup; // Should decode 5 values + if (n != 4) goto cleanup; // Should decode 4 values result = true; @@ -2693,8 +2693,8 @@ bool do_test_sql_pk_decode(void) { rc = sqlite3_cloudsync_init(db, NULL, NULL); if (rc != SQLITE_OK) goto cleanup; - // Create a primary key with multiple values - rc = sqlite3_prepare_v2(db, "SELECT cloudsync_pk_encode(123, 'hello', 3.14, X'DEADBEEF', NULL);", -1, &stmt, NULL); + // Create a primary key with multiple values (no NULL — primary keys cannot contain NULL) + rc = sqlite3_prepare_v2(db, "SELECT cloudsync_pk_encode(123, 'hello', 3.14, X'DEADBEEF');", -1, &stmt, NULL); if (rc != SQLITE_OK) goto cleanup; rc = sqlite3_step(stmt); @@ -2778,21 +2778,6 @@ bool do_test_sql_pk_decode(void) { sqlite3_finalize(stmt); stmt = NULL; - // Test cloudsync_pk_decode for NULL (index 5) - rc = sqlite3_prepare_v2(db, "SELECT cloudsync_pk_decode(?, 5);", -1, &stmt, NULL); - if (rc != SQLITE_OK) goto cleanup; - - rc = sqlite3_bind_blob(stmt, 1, pk_copy, pk_len, SQLITE_STATIC); - if (rc != SQLITE_OK) goto cleanup; - - rc = sqlite3_step(stmt); - if (rc != SQLITE_ROW) goto cleanup; - - if (sqlite3_column_type(stmt, 0) != SQLITE_NULL) goto cleanup; - - sqlite3_finalize(stmt); - stmt = NULL; - result = true; cleanup: From 9b6bd50358e30f99969605f357ec058ffd6048c5 Mon Sep 17 00:00:00 2001 From: Andrea Donetti Date: Tue, 10 Mar 2026 17:21:21 -0600 Subject: [PATCH 3/5] test: add null primary key rejection tests for SQLite and PostgreSQL Verify that INSERT operations with NULL primary key values are rejected with an appropriate error message, even when the column lacks an explicit NOT NULL constraint. --- test/postgresql/30_null_prikey_insert.sql | 68 +++++++++++++++++++++++ test/postgresql/full_test.sql | 1 + test/unit.c | 38 +++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 test/postgresql/30_null_prikey_insert.sql diff --git a/test/postgresql/30_null_prikey_insert.sql b/test/postgresql/30_null_prikey_insert.sql new file mode 100644 index 0000000..c7dc675 --- /dev/null +++ b/test/postgresql/30_null_prikey_insert.sql @@ -0,0 +1,68 @@ +-- Test: NULL Primary Key Insert Rejection +-- Verifies that inserting a NULL primary key into a cloudsync-enabled table fails +-- and that the metatable only contains rows for valid inserts. + +\set testid '30' +\ir helper_test_init.sql + +\connect postgres +\ir helper_psql_conn_setup.sql + +-- Cleanup and create test database +DROP DATABASE IF EXISTS cloudsync_test_30; +CREATE DATABASE cloudsync_test_30; + +\connect cloudsync_test_30 +\ir helper_psql_conn_setup.sql +CREATE EXTENSION IF NOT EXISTS cloudsync; + +-- Create table with primary key and init cloudsync +CREATE TABLE t_null_pk ( + id TEXT NOT NULL PRIMARY KEY, + value TEXT +); + +SELECT cloudsync_init('t_null_pk', 'CLS', true) AS _init \gset + +-- Test 1: INSERT with NULL primary key should fail +DO $$ +BEGIN + INSERT INTO t_null_pk (id, value) VALUES (NULL, 'test'); + RAISE EXCEPTION 'INSERT with NULL PK should have failed'; +EXCEPTION WHEN not_null_violation THEN + -- Expected +END $$; + +SELECT (COUNT(*) = 0) AS null_pk_rejected FROM t_null_pk \gset +\if :null_pk_rejected +\echo [PASS] (:testid) NULL PK insert rejected +\else +\echo [FAIL] (:testid) NULL PK insert was not rejected +SELECT (:fail::int + 1) AS fail \gset +\endif + +-- Test 2: INSERT with valid (non-NULL) primary key should succeed +INSERT INTO t_null_pk (id, value) VALUES ('valid_id', 'test'); + +SELECT (COUNT(*) = 1) AS valid_insert_ok FROM t_null_pk WHERE id = 'valid_id' \gset +\if :valid_insert_ok +\echo [PASS] (:testid) Valid PK insert succeeded +\else +\echo [FAIL] (:testid) Valid PK insert failed +SELECT (:fail::int + 1) AS fail \gset +\endif + +-- Test 3: Metatable should have exactly 1 row (from the valid insert only) +SELECT (COUNT(*) = 1) AS meta_row_ok FROM t_null_pk_cloudsync \gset +\if :meta_row_ok +\echo [PASS] (:testid) Metatable has exactly 1 row +\else +\echo [FAIL] (:testid) Metatable row count mismatch +SELECT (:fail::int + 1) AS fail \gset +\endif + +-- Cleanup +\ir helper_test_cleanup.sql +\if :should_cleanup +DROP DATABASE IF EXISTS cloudsync_test_30; +\endif diff --git a/test/postgresql/full_test.sql b/test/postgresql/full_test.sql index 0a260ba..279b937 100644 --- a/test/postgresql/full_test.sql +++ b/test/postgresql/full_test.sql @@ -37,6 +37,7 @@ \ir 27_rls_batch_merge.sql \ir 28_db_version_tracking.sql \ir 29_rls_multicol.sql +\ir 30_null_prikey_insert.sql -- 'Test summary' \echo '\nTest summary:' diff --git a/test/unit.c b/test/unit.c index 1157ee5..e3dd38a 100644 --- a/test/unit.c +++ b/test/unit.c @@ -2017,6 +2017,43 @@ bool do_test_error_cases (sqlite3 *db) { return true; } +bool do_test_null_prikey_insert (sqlite3 *db) { + // Create a table with a primary key that allows NULL (no NOT NULL constraint) + const char *sql = "CREATE TABLE IF NOT EXISTS t_null_pk (id TEXT PRIMARY KEY, value TEXT);" + "SELECT cloudsync_init('t_null_pk');"; + int rc = sqlite3_exec(db, sql, NULL, NULL, NULL); + if (rc != SQLITE_OK) return false; + + // Attempt to insert a row with NULL primary key — should fail + char *errmsg = NULL; + sql = "INSERT INTO t_null_pk (id, value) VALUES (NULL, 'test');"; + rc = sqlite3_exec(db, sql, NULL, NULL, &errmsg); + if (rc == SQLITE_OK) return false; // should have failed + if (!errmsg) return false; + + // Verify the error message matches the expected format + const char *expected = "Insert aborted because primary key in table t_null_pk contains NULL values."; + bool match = (strcmp(errmsg, expected) == 0); + sqlite3_free(errmsg); + if (!match) return false; + + // Verify that a non-NULL primary key insert succeeds + sql = "INSERT INTO t_null_pk (id, value) VALUES ('valid_id', 'test');"; + rc = sqlite3_exec(db, sql, NULL, NULL, NULL); + if (rc != SQLITE_OK) return false; + + // Verify the metatable has exactly 1 row (only the valid insert) + sqlite3_stmt *stmt = NULL; + rc = sqlite3_prepare_v2(db, "SELECT COUNT(*) FROM t_null_pk_cloudsync;", -1, &stmt, NULL); + if (rc != SQLITE_OK) return false; + if (sqlite3_step(stmt) != SQLITE_ROW) { sqlite3_finalize(stmt); return false; } + int count = sqlite3_column_int(stmt, 0); + sqlite3_finalize(stmt); + if (count != 1) return false; + + return true; +} + bool do_test_internal_functions (void) { sqlite3 *db = NULL; sqlite3_stmt *vm = NULL; @@ -7842,6 +7879,7 @@ int main (int argc, const char * argv[]) { result += test_report("DBUtils Test:", do_test_dbutils()); result += test_report("Minor Test:", do_test_others(db)); result += test_report("Test Error Cases:", do_test_error_cases(db)); + result += test_report("Null PK Insert Test:", do_test_null_prikey_insert(db)); result += test_report("Test Single PK:", do_test_single_pk(print_result)); int test_mask = TEST_INSERT | TEST_UPDATE | TEST_DELETE; From 47acb91a11649e2fd1dc05d3fdbb50dfeb06989a Mon Sep 17 00:00:00 2001 From: Andrea Donetti Date: Tue, 10 Mar 2026 17:21:42 -0600 Subject: [PATCH 4/5] chore: bump version --- src/cloudsync.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cloudsync.h b/src/cloudsync.h index 11aed15..94a9410 100644 --- a/src/cloudsync.h +++ b/src/cloudsync.h @@ -17,7 +17,7 @@ extern "C" { #endif -#define CLOUDSYNC_VERSION "0.9.115" +#define CLOUDSYNC_VERSION "0.9.116" #define CLOUDSYNC_MAX_TABLENAME_LEN 512 #define CLOUDSYNC_VALUE_NOTSET -1 From 508b4c7f7a3a84908a7837b734b299056e835c66 Mon Sep 17 00:00:00 2001 From: Andrea Donetti Date: Tue, 10 Mar 2026 17:23:03 -0600 Subject: [PATCH 5/5] docs: remove NOT NULL requirement from primary key definitions The extension now enforces NULL primary key rejection at runtime, so the explicit NOT NULL constraint on PK columns is no longer a schema requirement. Replace the "must be NOT NULL" guidance with a note about runtime enforcement. --- .../test-sync-roundtrip-postgres-local.md | 6 +++--- .../test-sync-roundtrip-postrges-local-rls.md | 6 +++--- .../test-sync-roundtrip-sqlitecloud-rls.md | 6 +++--- API.md | 2 +- README.md | 8 ++++---- docs/postgresql/CLIENT.md | 16 ++++++++-------- docs/postgresql/RLS.md | 6 +++--- docs/postgresql/SUPABASE.md | 2 +- examples/simple-todo-db/README.md | 6 +++--- 9 files changed, 29 insertions(+), 29 deletions(-) diff --git a/.claude/commands/test-sync-roundtrip-postgres-local.md b/.claude/commands/test-sync-roundtrip-postgres-local.md index 686fc12..8713466 100644 --- a/.claude/commands/test-sync-roundtrip-postgres-local.md +++ b/.claude/commands/test-sync-roundtrip-postgres-local.md @@ -16,7 +16,7 @@ Ask the user to provide a DDL query for the table(s) to test. It can be in Postg **Option 1: Simple TEXT primary key** ```sql CREATE TABLE test_sync ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, name TEXT, value INTEGER ); @@ -34,13 +34,13 @@ CREATE TABLE test_uuid ( **Option 3: Two tables scenario (tests multi-table sync)** ```sql CREATE TABLE authors ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, name TEXT, email TEXT ); CREATE TABLE books ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, title TEXT, author_id TEXT, published_year INTEGER diff --git a/.claude/commands/test-sync-roundtrip-postrges-local-rls.md b/.claude/commands/test-sync-roundtrip-postrges-local-rls.md index 94fa886..ef74646 100644 --- a/.claude/commands/test-sync-roundtrip-postrges-local-rls.md +++ b/.claude/commands/test-sync-roundtrip-postrges-local-rls.md @@ -16,7 +16,7 @@ Ask the user to provide a DDL query for the table(s) to test. It can be in Postg **Option 1: Simple TEXT primary key with user_id for RLS** ```sql CREATE TABLE test_sync ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, user_id UUID NOT NULL, name TEXT, value INTEGER @@ -36,14 +36,14 @@ CREATE TABLE test_uuid ( **Option 3: Two tables scenario with user ownership** ```sql CREATE TABLE authors ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, user_id UUID NOT NULL, name TEXT, email TEXT ); CREATE TABLE books ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, user_id UUID NOT NULL, title TEXT, author_id TEXT, diff --git a/.claude/commands/test-sync-roundtrip-sqlitecloud-rls.md b/.claude/commands/test-sync-roundtrip-sqlitecloud-rls.md index 4adb700..0bf36c0 100644 --- a/.claude/commands/test-sync-roundtrip-sqlitecloud-rls.md +++ b/.claude/commands/test-sync-roundtrip-sqlitecloud-rls.md @@ -16,7 +16,7 @@ Ask the user to provide a DDL query for the table(s) to test. It can be in Postg **Option 1: Simple TEXT primary key with user_id for RLS** ```sql CREATE TABLE test_sync ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, user_id TEXT NOT NULL, name TEXT, value INTEGER @@ -26,14 +26,14 @@ CREATE TABLE test_sync ( **Option 2: Two tables scenario with user ownership** ```sql CREATE TABLE authors ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, user_id TEXT NOT NULL, name TEXT, email TEXT ); CREATE TABLE books ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, user_id TEXT NOT NULL, title TEXT, author_id TEXT, diff --git a/API.md b/API.md index cd13a1b..f8f9c37 100644 --- a/API.md +++ b/API.md @@ -41,8 +41,8 @@ This document provides a reference for the SQLite functions provided by the `sql Before initialization, `cloudsync_init` performs schema sanity checks to ensure compatibility with CRDT requirements and best practices. These checks include: - Primary keys should not be auto-incrementing integers; GUIDs (UUIDs, ULIDs) are highly recommended to prevent multi-node collisions. -- All primary key columns must be `NOT NULL`. - All non-primary key `NOT NULL` columns must have a `DEFAULT` value. +- **Note:** Any write operation that includes a NULL value for a primary key column will be rejected with an error, even if SQLite would normally allow it due to a legacy behavior. **Schema Design Considerations:** diff --git a/README.md b/README.md index ad91448..b01226b 100644 --- a/README.md +++ b/README.md @@ -264,7 +264,7 @@ sqlite3 myapp.db -- Create a table (primary key MUST be TEXT for global uniqueness) CREATE TABLE IF NOT EXISTS my_data ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, value TEXT NOT NULL DEFAULT '', created_at TEXT DEFAULT CURRENT_TIMESTAMP ); @@ -313,7 +313,7 @@ SELECT cloudsync_terminate(); -- Load extension and create identical table structure .load ./cloudsync CREATE TABLE IF NOT EXISTS my_data ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, value TEXT NOT NULL DEFAULT '', created_at TEXT DEFAULT CURRENT_TIMESTAMP ); @@ -372,12 +372,12 @@ When designing your database schema for SQLite Sync, follow these best practices - **Use globally unique identifiers**: Always use TEXT primary keys with UUIDs, ULIDs, or similar globally unique identifiers - **Avoid auto-incrementing integers**: Integer primary keys can cause conflicts across multiple devices - **Use `cloudsync_uuid()`**: The built-in function generates UUIDv7 identifiers optimized for distributed systems -- **All primary keys must be explicitly declared as `NOT NULL`**. +- **Note:** Any write operation that includes a NULL value for a primary key column will be rejected with an error, even if SQLite would normally allow it due to a legacy behavior. ```sql -- ✅ Recommended: Globally unique TEXT primary key CREATE TABLE users ( - id TEXT PRIMARY KEY NOT NULL, -- Use cloudsync_uuid() + id TEXT PRIMARY KEY, -- Use cloudsync_uuid() name TEXT NOT NULL, email TEXT UNIQUE NOT NULL ); diff --git a/docs/postgresql/CLIENT.md b/docs/postgresql/CLIENT.md index 9ef8cc8..58751d1 100644 --- a/docs/postgresql/CLIENT.md +++ b/docs/postgresql/CLIENT.md @@ -34,8 +34,8 @@ so CloudSync can sync between a PostgreSQL server and SQLite clients. ### 1) Primary Keys -- Use **TEXT NOT NULL** primary keys in SQLite. -- PostgreSQL primary keys can be **TEXT NOT NULL** or **UUID**. If the PK type +- Use **TEXT** primary keys in SQLite. +- PostgreSQL primary keys can be **TEXT** or **UUID**. If the PK type isn't explicitly mapped to a DBTYPE (like UUID), it will be converted to TEXT in the payload so it remains compatible with the SQLite extension. - Generate IDs with `cloudsync_uuid()` on both sides. @@ -43,17 +43,17 @@ so CloudSync can sync between a PostgreSQL server and SQLite clients. SQLite: ```sql -id TEXT PRIMARY KEY NOT NULL +id TEXT PRIMARY KEY ``` PostgreSQL: ```sql -id TEXT PRIMARY KEY NOT NULL +id TEXT PRIMARY KEY ``` PostgreSQL (UUID): ```sql -id UUID PRIMARY KEY NOT NULL +id UUID PRIMARY KEY ``` ### 2) NOT NULL Columns Must Have DEFAULTs @@ -99,7 +99,7 @@ Use defaults that serialize the same on both sides: SQLite: ```sql CREATE TABLE notes ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, title TEXT NOT NULL DEFAULT '', body TEXT DEFAULT '', views INTEGER NOT NULL DEFAULT 0, @@ -111,7 +111,7 @@ CREATE TABLE notes ( PostgreSQL: ```sql CREATE TABLE notes ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, title TEXT NOT NULL DEFAULT '', body TEXT DEFAULT '', views INTEGER NOT NULL DEFAULT 0, @@ -136,7 +136,7 @@ SELECT cloudsync_init('notes'); ### Checklist -- [ ] PKs are TEXT + NOT NULL +- [ ] PKs are TEXT (or UUID in PostgreSQL) - [ ] All NOT NULL columns have DEFAULT - [ ] Only INTEGER/FLOAT/TEXT/BLOB-compatible types - [ ] Same column names and order diff --git a/docs/postgresql/RLS.md b/docs/postgresql/RLS.md index c0871d3..cc686ad 100644 --- a/docs/postgresql/RLS.md +++ b/docs/postgresql/RLS.md @@ -31,7 +31,7 @@ Given a table with an ownership column (`user_id`): ```sql CREATE TABLE documents ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, user_id UUID, title TEXT, content TEXT @@ -68,13 +68,13 @@ This example shows the complete flow of syncing data between two databases where ```sql -- Source database (DB A) — no RLS, represents the sync server CREATE TABLE documents ( - id TEXT PRIMARY KEY NOT NULL, user_id UUID, title TEXT, content TEXT + id TEXT PRIMARY KEY, user_id UUID, title TEXT, content TEXT ); SELECT cloudsync_init('documents'); -- Target database (DB B) — RLS enforced CREATE TABLE documents ( - id TEXT PRIMARY KEY NOT NULL, user_id UUID, title TEXT, content TEXT + id TEXT PRIMARY KEY, user_id UUID, title TEXT, content TEXT ); SELECT cloudsync_init('documents'); ALTER TABLE documents ENABLE ROW LEVEL SECURITY; diff --git a/docs/postgresql/SUPABASE.md b/docs/postgresql/SUPABASE.md index 94aa466..a800ae3 100644 --- a/docs/postgresql/SUPABASE.md +++ b/docs/postgresql/SUPABASE.md @@ -76,7 +76,7 @@ SELECT cloudsync_version(); ```sql CREATE TABLE notes ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, body TEXT DEFAULT '' ); diff --git a/examples/simple-todo-db/README.md b/examples/simple-todo-db/README.md index 772b8fe..55a639c 100644 --- a/examples/simple-todo-db/README.md +++ b/examples/simple-todo-db/README.md @@ -59,7 +59,7 @@ Tables must be created on both the local database and SQLite Cloud with identica -- Create the main tasks table -- Note: Primary key MUST be TEXT (not INTEGER) for global uniqueness CREATE TABLE IF NOT EXISTS tasks ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, userid TEXT NOT NULL DEFAULT '', title TEXT NOT NULL DEFAULT '', description TEXT DEFAULT '', @@ -84,7 +84,7 @@ SELECT cloudsync_is_enabled('tasks'); - Execute the same CREATE TABLE statement: ```sql CREATE TABLE IF NOT EXISTS tasks ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, userid TEXT NOT NULL DEFAULT '', title TEXT NOT NULL DEFAULT '', description TEXT DEFAULT '', @@ -149,7 +149,7 @@ sqlite3 todo_device_b.db ```sql -- Create identical table structure CREATE TABLE IF NOT EXISTS tasks ( - id TEXT PRIMARY KEY NOT NULL, + id TEXT PRIMARY KEY, userid TEXT NOT NULL DEFAULT '', title TEXT NOT NULL DEFAULT '', description TEXT DEFAULT '',