Infer sync statements for raw tables#164
Infer sync statements for raw tables#164simolus3 wants to merge 4 commits intoraw-table-triggersfrom
Conversation
rkistner
left a comment
There was a problem hiding this comment.
Is it feasible to add a test for the generated SQL? Even if it's just a Rust unit test rather than an "end-to-end" Dart one.
| final table = { | ||
| 'name': 'users', | ||
| 'table_name': 'local_users', | ||
| 'local_only_columns': ['local'], |
There was a problem hiding this comment.
I'm wondering, should we perhaps invert this? Instead of specifying the local_only_columns, rather specify a specific list of columns to sync?
That would be closer to what we have with JSON tables (except we don't need the types).
And we could still default to inferring all the columns if not specified.
| let mut buffer = SqlBuffer::new(); | ||
| let mut params = vec![]; | ||
|
|
||
| buffer.push_str("INSERT OR REPLACE INTO "); |
There was a problem hiding this comment.
Could we perhaps change this to use upsert statements instead?
This is specifically required when using local-only columns - see https://docs.powersync.com/client-sdks/advanced/raw-tables#use-upsert-instead-of-insert-or-replace
However, even without local-only columns it could be better, especially if the developer users custom triggers (REPLACE triggers a delete + insert if the record exists, while upsert just triggers an update).
It may even be useful to change or JSON tables to use upserts, but that may need a separate investigation on the implications.
As far as I know, the only advantage to using a REPLACE statement is to simplify the query (less duplication of column names), which is not relevant when we're generating it.
This builds on top of the new raw table options introduced in #163.
This makes the
putanddeletestatements set on raw tables optional. If they're omitted, thesync_localstep of each sync will attempt to infer them by looking at the actual columns of the table:idcolumn will be set to the affected row id.Inferred statements are cached until
pragma schema_versionreturns a new value.For now, this does not support a dedicated
_restcolumn that would be inferred by default. Users interested in that would have to use explicit statements instead.