Generated SQL from createCustomSqlitePersister nulls out other columns

[LucDeCaf]

Describe the bug

When using store.setCell() on an existing cell, the expected behaviour is that the cell will have its value changed and the rest of the row remains unchanged. However, when using a custom SQLite persister with orReplace=1 set, the generated SQL will look as such:

INSERT OR REPLACE "table_name" ("_id","col_to_change") VALUES (?,?);

The issue with this SQL is that every other column in the updated row will be set to NULL, which is probably not expected or desired behaviour.

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

1. Create a tabular store with some data
2. Create a SQLite persister that uses orReplace=1 (I used PowerSync)
3. Use setCell to update a single cell in the store
4. Generated SQL will null out all fields except replaced field and PK

Expected behavior

The expected behaviour is that the cell that is changed will be updated and the remaining cells will not be set to null. The generated SQL could look something like this:

INSERT OR REPLACE "table_name" ("_id","col_to_change","other_col_1","other_col_2","...") VALUES (?,?,?,?,...);

Screenshots or Videos

No response

Platform

• OS: macOS 14.5
• Browser: Chrome
• Version: Tinybase v5.3.1

Additional context

No response

[waynesbrain]

Hi @LucDeCaf I was just reading about this out of curiosity and it looks like TinyBase is using Method # 2 of this answer on how to do UPSERTs with SQLite.

I believe this is the relevant code in TinyBase e.g.

INSERT INTO Book (ID, Name)
  VALUES (1001, 'SQLite')
  ON CONFLICT (ID) DO
  UPDATE SET Name=excluded.Name;

According to that answer it should be working. Hmmm...

[waynesbrain]

Oh I see the problem here

    (orReplace
        ? EMPTY_STRING
        : 'ON CONFLICT(' +

It does NOT apply the ON CONFLICT clause when orReplace is truthy.

[LucDeCaf]

@waynesbrain Yeah, I saw the ON CONFLICT clause - The problem with relying on ON CONFLICT is that some SQLite persisters (eg. PowerSync) don't support the ON CONFLICT (...) UPDATE syntax, which is why I think the orReplace flag exists.

[waynesbrain]

Okay I see, I misread the SO answer and I thought it was using OR REPLACE as well as ON CONFLICT within the same statement, and so the TinyBase code confused me, but that was incorrect.

So the change you're asking for is basically, when calling setCell and orReplace is enabled:

• Set all of the fields for that table to their current value,
• except setCell target field which gets a new value.

Is that correct?

I guess the workaround for this would be to make your own upsert function using TinyBase setRow.

[LucDeCaf]

That's pretty much what I expect, although I wonder how it would work when setCell is used with a rowId that doesn't exist yet. The current workaround I found is to just use setRow, but it would be nice if this workaround wasn't required or if this behaviour was better documented (or maybe I just didn't see the docs for this).

[jamesgpearce]

You're correct that orReplace only exists to support PowerSync (so we should check in with @bndkt). PowerSync is quite heavily mocked so it's possible we aren't properly testing this.

But I can also take a look at this. Need to get PowerSync installed I guess!

[jamesgpearce]

Unfortunately the PowerSync website does not want to help out right now 🤣

[waynesbrain]

@jamesgpearce You can self host it in a Docker. (Also, the powersync.com site is working for me but I see it's the same company or related to JourneyApps you showed there.)

• https://docs.powersync.com/self-hosting/getting-started
  • https://github.com/powersync-ja/powersync-service
  • https://www.powersync.com/open-source
    • https://github.com/powersync-ja/powersync-js

[jamesgpearce]

I tried and failed with docker for reasons I didn't quite have time to debug. But I figure I can just hardwire the tests onto the wa-sqlite module to repro. brb.

[benitav]

Unfortunately the PowerSync website does not want to help out right now 🤣

@jamesgpearce Bad timing 🙈 Our sign-up page had an issue over the weekend - we fixed it a couple of hours ago, so you should be able to sign up to PowerSync now.

[LucDeCaf]

@jamesgpearce I’m down to create a PR with a fix - I’ve spent a while debugging the persister already and have some fixes in mind, as well as a (slightly clunky) PowerSync app to test with

[jamesgpearce]

Super cool! I also remocked the powersync tests with the JA wa-sqlite fork. Just got to finish off the table watching hook and we should get much higher fidelity coverage too.