Sandbox::https://github.com/powersync-ja/powersync-sqlite-core/pull/236

khawarizmus · 2026-11-12T08:05:56Z

Problem

When a synced row already exists, core writes it with REPLACE INTO. SQLite then:

deletes the old row and inserts a new one with a new rowid
runs DELETE triggers only if recursive_triggers is on (JS and Dart native turn it on; Swift, Kotlin and Dart web don't)

Without the pragma, triggers on ps_data__* tables break. Example: FTS shows outdated/duplicate results and inserts fail with constraint failed.
Reported in powersync-ja/powersync-swift#93 and thunderbird/thunderbolt#1512.

Fix

Use an upsert (INSERT ... ON CONFLICT DO UPDATE) for ps_data__* and ps_untyped. Direct and raw tables already do this.

Why not turn on `recursive_triggers` in every SDK?

	`recursive_triggers`	Upsert
Where	Every SDK and every connection	Once, in core
Synced edit looks like	DELETE + INSERT	UPDATE
Rowid	Changes	Stays the same
Cascading-delete triggers	Fire on every edit	Fire only on real deletes

Keep the pragma in JS/Dart: user-defined raw table put statements may still use REPLACE.

Behavior change

A synced edit to an existing row now runs UPDATE triggers and keeps the rowid, on every SDK.
Triggers that only listen for INSERT or DELETE no longer see synced edits. This needs an upgrade note.

JS diff triggers (trackTableDiff) are affected. A synced edit was a DELETE + INSERT pair, and is now one UPDATE with previous_value:

tracking INSERT only: synced edits no longer appear, so add UPDATE
tracking DELETE: no more false deletes on edits
tracking UPDATE: synced edits now appear

Apps without custom triggers on internal tables: no change.

Tests

Added: a synced update runs the UPDATE trigger and keeps the rowid (ps_data__* and ps_untyped).

Open question

Release in a minor version with an upgrade note, or in the next major?

Used Claude to plan and execute the fix.

rkistner

I'm in favor of this change, and think a minor version update should be fine.

It would however be good to do a basic performance test of this. These two benchmarks should help here:

powersync-sqlite-core/dart/test/sync_local_performance_test.dart

Lines 134 to 149 in d236de9

    
           test('sync_local (full)', () { 
        
             final timer = runSyncLocal(null); 
        
             // These are fairly generous limits, to catch significant regressions only. 
        
             expect(vfs.tempWrites, lessThan(count / 50)); 
        
             expect(timer.elapsed, 
        
                 lessThan(Duration(milliseconds: 100 + (count / 50).round()))); 
        
           }); 
        
           test('sync_local (partial)', () { 
        
             final timer = runSyncLocal(2); 
        
             expect(vfs.tempWrites, lessThan(count / 50)); 
        
             expect(timer.elapsed, 
        
                 lessThan(Duration(milliseconds: 100 + (count / 50).round()))); 
        
           });

. They measure number of filesystem operations rather than actual throughput, but it should give a good indication of potential performance impact.

I also think it would be good to additionally enable recursive triggers in all our SDKs, since it's quite unexpected if triggers don't fire after synced changes.

It would be good to get @simolus3's input on this as well.

simolus3 · 2026-11-12T14:35:07Z

I agree that using an upsert here makes sense, and that we can use a minor/patch version (changing the behavior of diff triggers should be okay since it's an improvement and they're not stable yet).

I also think it would be good to additionally enable recursive triggers in all our SDKs, since it's quite unexpected if triggers don't fire after synced changes.

Yeah, judging by the fact that Dart/native enables this and Dart/web doesn't, I think the missing SDKs are an oversight.

Test results:

Test name	`main`	`fix/sync-local-upsert`
unique ids full	`Reads: 1290591 + 0 ; Writes: 69031 + 0`	`Reads: 1291626 + 0 ; Writes: 69031 + 0`
unique ids partial	`Reads: 521257 + 0 ; Writes: 27613 + 0`	`Reads: 521340 + 0 ; Writes: 27613 + 0`
duplicate ids full	`Reads: 431637 + 0 ; Writes: 38 + 0`	`Reads: 431637 + 0 ; Writes: 38 + 0`
duplicate ids partial	`Reads: 199397 + 0 ; Writes: 34 + 0`	`Reads: 199397 + 0 ; Writes: 34 + 0`
small number of changes full	`Reads: 5680 + 0 ; Writes: 736 + 0`	`Reads: 5681 + 0 ; Writes: 736 + 0`
small number of changes partial	`Reads: 2958 + 0 ; Writes: 312 + 0`	`Reads: 2988 + 0 ; Writes: 312 + 0`
large number of buckets full	`Reads: 116062 + 0 ; Writes: 12729 + 0`	`Reads: 116113 + 0 ; Writes: 12729 + 0`
large number of buckets partial	`Reads: 863 + 0 ; Writes: 83 + 0`	`Reads: 865 + 0 ; Writes: 83 + 0`

The slight regression in the amount of reads doesn't seem that concerning to me.

Use upsert instead of REPLACE INTO for synced rows

828d3d2

khawarizmus requested review from Chriztiaan, rkistner and simolus3 November 12, 2026 08:05

rkistner reviewed Nov 12, 2026

View reviewed changes

Navigation Menu

Repository navigation

Use upsert instead of REPLACE INTO when applying synced rows - #236

Use upsert instead of REPLACE INTO when applying synced rows#236
khawarizmus wants to merge 1 commit into
mainfrom
fix/sync-local-upsert

khawarizmus commented Nov 12, 2026

Uh oh!

rkistner left a comment

Uh oh!

simolus3 commented Nov 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	test('sync_local (full)', () {
	final timer = runSyncLocal(null);

	// These are fairly generous limits, to catch significant regressions only.
	expect(vfs.tempWrites, lessThan(count / 50));
	expect(timer.elapsed,
	lessThan(Duration(milliseconds: 100 + (count / 50).round())));
	});

	test('sync_local (partial)', () {
	final timer = runSyncLocal(2);

	expect(vfs.tempWrites, lessThan(count / 50));
	expect(timer.elapsed,
	lessThan(Duration(milliseconds: 100 + (count / 50).round())));
	});

Conversation

khawarizmus commented Nov 12, 2026

Problem

Fix

Why not turn on recursive_triggers in every SDK?

Behavior change

Tests

Open question

Uh oh!

rkistner left a comment

Choose a reason for hiding this comment

Uh oh!

simolus3 commented Nov 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Why not turn on `recursive_triggers` in every SDK?