Skip to content

Simpler crud vtab, part 1 #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Simpler crud vtab, part 1 #89

wants to merge 10 commits into from

Conversation

simolus3
Copy link
Contributor

To automatically associate local writes with a transaction id, we use a powersync_crud_ eponymous-only virtual table tracking started transactions and forwarding writes to ps_crud.

But for most writes (the only exception are inserts to insert-only tables), writes actually involve three steps:

  1. Creating a ps_crud entry through the vtab.
  2. Marking the row as modified in ps_updated_rows.
  3. Marking the $local bucket as dirty.

Since the last two steps are almost always the same, this introduces a new powersync_crud virtual table (without a trailing underscore) that behaves almost identical to powersync_crud_, but:

  1. Instead of taking a premade JSON string as parameter, it takes the individual fields (e.g. op, id, type, data, old data, metadata).
  2. It uses these individual columns to update ps_updated_rows for the correct row / type.
  3. It updates $local.

A future step is to make powersync_crud inserts a nop during sync_local, simplifying custom triggers for raw tables.

@simolus3 simolus3 requested a review from rkistner June 11, 2025 16:15
@simolus3
Copy link
Contributor Author

I don't think this needs a special migration because the old triggers will continue to work. And since powersync_replace_schema does a diff between the expected and actual triggers after an update, it would just replace them on initialization. But as discussed offline, an empty up migration with a down migration to drop triggers on a core downgrade might be useful because the new powersync_crud vtab wouldn't exist anymore.

There are some failures in the migration tests - mainly because the triggers at older versions look different to the triggers now. I think the only good way to fix these tests is to expect the current trigger definitions everywhere (which is a bit odd when e.g. testing a migration from version 3 to 5, but I don't think the triggers itself should be versioned here).

@simolus3 simolus3 marked this pull request as ready for review June 12, 2025 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant