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 '', 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/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 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/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 1caba38..e3dd38a 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; @@ -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; @@ -2225,8 +2262,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 +2284,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 +2730,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 +2815,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: @@ -7857,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;