feat: first implementation of batch merge and rls, remove the callback previously used to approve changes using the RLS select rule

Author: andinux

plans/BATCH_MERGE_AND_RLS.md

@@ -0,0 +1,80 @@
+# Deferred Column-Batch Merge and RLS Support
+
+## Problem
+
+CloudSync resolves CRDT conflicts per-column, so `cloudsync_payload_apply` processes column changes one at a time. Previously each winning column was written immediately via a single-column `INSERT ... ON CONFLICT DO UPDATE`. This caused two issues with PostgreSQL RLS:
+
+1. **Partial-column UPSERT fails INSERT WITH CHECK**: An update to just `title` generates `INSERT INTO docs (id, title) VALUES (...)  ON CONFLICT DO UPDATE SET title=...`. PostgreSQL evaluates the INSERT `WITH CHECK` policy *before* checking for conflicts. Missing columns (e.g. `user_id`) default to NULL, so `auth.uid() = user_id` fails. The ON CONFLICT path is never reached.
+
+2. **Premature flush in SPI**: `database_in_transaction()` always returns true inside PostgreSQL SPI. The old code only updated `last_payload_db_version` inside `if (!in_transaction && db_version_changed)`, so the variable stayed at -1, `db_version_changed` was true on every row, and batches flushed after every single column.
+
+## Solution
+
+### Batch merge (`merge_pending_batch`)
+
+New structs in `cloudsync.c`:
+
+- `merge_pending_entry` — one buffered column (col_name, col_value via `database_value_dup`, col_version, db_version, site_id, seq)
+- `merge_pending_batch` — collects entries for one PK (table, pk, row_exists flag, entries array)
+
+`data->pending_batch` is set to `&batch` (stack-allocated) at the start of `cloudsync_payload_apply`. The INSTEAD OF trigger calls `merge_insert`, which calls `merge_pending_add` instead of `merge_insert_col`. Flush happens at PK/table/db_version boundaries and after the loop.
+
+### UPDATE vs UPSERT (`row_exists` flag)
+
+`merge_insert` sets `batch->row_exists = (local_cl != 0)` on the first winning column. At flush time `merge_flush_pending` selects:
+
+- `row_exists=true` -> `sql_build_update_pk_and_multi_cols` -> `UPDATE docs SET title=$2::text WHERE id=$1::text`
+- `row_exists=false` -> `sql_build_upsert_pk_and_multi_cols` -> `INSERT ... ON CONFLICT DO UPDATE`
+
+For SQLite, `sql_build_update_pk_and_multi_cols` delegates to the UPSERT builder (no RLS).
+
+### `last_payload_db_version` fix
+
+Moved the update outside the savepoint block so it executes unconditionally:
+
+```c
+if (db_version_changed) {
+    last_payload_db_version = decoded_context.db_version;
+}
+```
+
+Previously this was inside `if (!in_transaction && db_version_changed)`, which never ran in SPI.
+
+## SPI and Memory Management
+
+### Nested SPI levels
+
+`pg_cloudsync_payload_apply` calls `SPI_connect` (level 1). Inside the loop, `databasevm_step` executes `INSERT INTO cloudsync_changes`, which fires the INSTEAD OF trigger. The trigger calls `SPI_connect` (level 2), runs `merge_insert` / `merge_pending_add`, then `SPI_finish` back to level 1. The deferred `merge_flush_pending` runs at level 1.
+
+### `database_in_transaction()` in SPI
+
+Always returns true in SPI context. No savepoints are created. This is why `last_payload_db_version` must be updated unconditionally — the savepoint-gated update path is dead code in PostgreSQL.
+
+### Error handling in SPI
+
+When RLS denies a write, PostgreSQL raises an error inside SPI which is caught by `PG_CATCH()` in `databasevm_step`. Since there are no savepoints, an RLS denial aborts the current SPI transaction for subsequent SQL within that `cloudsync_payload_apply` call.
+
+### Batch cleanup paths
+
+`batch.entries` is heap-allocated via `cloudsync_memory_realloc` and reused across flushes. Each entry's `col_value` (from `database_value_dup`) is freed by `merge_pending_free_entries` on every flush. The entries array itself is freed once at the end of `cloudsync_payload_apply`. Error paths (`goto cleanup`, early returns) must call `merge_pending_free_entries` before freeing the array to avoid leaking `col_value` copies.
+
+## Files Changed
+
+| File | Change |
+|------|--------|
+| `src/cloudsync.c` | Batch merge structs, `merge_pending_add`, `merge_flush_pending`, `merge_pending_free_entries`; `pending_batch` field on context; `row_exists` propagation in `merge_insert`; batch mode in `merge_sentinel_only_insert`; `last_payload_db_version` fix; removed `payload_apply_callback` |
+| `src/cloudsync.h` | Removed `CLOUDSYNC_PAYLOAD_APPLY_STEPS` enum |
+| `src/database.h` | Added `sql_build_upsert_pk_and_multi_cols`, `sql_build_update_pk_and_multi_cols`; removed callback typedefs |
+| `src/sqlite/database_sqlite.c` | Implemented `sql_build_upsert_pk_and_multi_cols` (dynamic SQL); `sql_build_update_pk_and_multi_cols` (delegates to upsert); removed callback functions |
+| `src/postgresql/database_postgresql.c` | Implemented `sql_build_update_pk_and_multi_cols` (meta-query against `pg_catalog` generating typed UPDATE) |
+| `test/unit.c` | Removed callback code and `do_test_andrea` debug function |
+| `test/postgresql/27_rls_batch_merge.sql` | Tests 1-3 (superuser) + Tests 4-6 (authenticated-role RLS enforcement) |
+| `docs/postgresql/RLS.md` | Documented INSERT vs UPDATE paths and partial-column RLS interaction |
+
+## TODO:
+
+ - check the working logs on test psql:test/postgresql/27_rls_batch_merge.sql:246: WARNING:  resource was not closed: relation "documents_pkey"
+ - fully implement sql_build_update_pk_and_multi_cols in the sqlite extension because sqlitecloud has RLS even if sqlite doesn't
+ - the batch apply is better than single apply even if rls is not set? for example, in sqlite client there is no RLS, should we completely exclude this new code and follow the old path or it is still better to use this batch apply path? pros/cons
+ - there is still an issue of postgres rollbacking the full apply transaction if a change apply is denied by RLS because of the savepoints are not used inside transactions?
+ - add a new test like the n° 27 with more columns and more cases