Added a new compilation macro CLOUDSYNC_CHECK_NOTNULL_PRIKEYS (disabled by default)

Author: marcobambini

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.

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;

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);

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) {

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);

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: