Skip to content

Conversation

@JanezStupar
Copy link
Contributor

Hi Daniel,

I have had this issue reported on drift_crdt a while ago. The issue is that sql_crdt does not support INSERT - RETURNING semantic.

Instead of only fixing it in drift_crdt I thought this would be something that could improve sql_crdt.

I will be frank, I don't think the solution is particularly elegant. But I guess even from standpoint of SQL it is not a particularly elegant requirement in the first place.

The reason I put the logic into the query function is that the nature of the feature is such that values have to be returned after insert. And I guess this matches the upstream API's better.

Let me know if you see a better way forward.

@cachapa
Copy link
Owner

cachapa commented Oct 10, 2025

Hi Janez,

I'm honestly pretty skeptical about this one. It's adding a lot of code (and some of it duplicated) to support a very niche semantic.

Also, I don't currently have the bandwidth to properly evaluate this functionality, or to maintain it if it goes into the code so I prefer to leave this open for now so I can pick it up if it makes sense later.

In any case I would encourage you to implement it on your own package so you're not dependent on my free time.

@JanezStupar
Copy link
Contributor Author

I completely understand. It is a weird one. I don't even need it personally I only went and implemented it for completeness sake.

Do leave it open and if it ever speaks out to you, get back to me.

@JanezStupar
Copy link
Contributor Author

Hey Daniel. Its me again. I am closing this PR, because as I have noted this is a bad solution on the face of it.

I came up with a better solution, but it has quite a bit to it.

TLDR: I am working on postgres support for drift_crdt and during that I discovered a number of rough edges in the sql_crdt, sqlite_crdt and postgres_crdt libraries that make them somewhat incompatible with postgres.

So I kept picking at it until drift test suite passed for postgres.

There is quite a bit of changes, everything is documented. All existing tests pass and I wrote additional tests for the new features/fixes.

I can appreciate that you are busy and that you need time to decide whether to take these changes in - although IMO they do make libraries better without breaking anything.

So I decided to give you a couple of months to make a call whether to merge these changes back in and release them as v4.0.0 and if you decline I will release them as a fork.

Here is a summary of changes made and link to respective branches:

sql_crdt

  • Changed execute return values from void to ExecuteResult, which can be either VoidResult or QueryResult. This is a more elegant way of enabling execute to return values - facilitating the ... RETURNING operations

  • Updated query parsing tooling to manage Postgres style placeholders '$N' seamlessly.

  • Fixed 'transformAutomaticExplicitSql' function so that it works with Postgres style delimited identifiers.

  • Bumped to version 4.0.0, due to breaking changes

https://github.com/JanezStupar/sql_crdt/tree/feature/postgres_related

sqlite_crdt

  • Bumped to version 4.0.0, due to breaking changes in sql_crdt API - although the backwards compatibility is maintained for execute calls - which do not inspect returned values (why would you catch void anyway?).

https://github.com/JanezStupar/sqlite_crdt/tree/feature/ver4

postgres_crdt

  • Bumped to version 4.0.0, due to breaking changes in sql_crdt API - although the backwards compatibility is maintained for execute calls - which do not inspect returned values (why would you catch void anyway?).

  • While postgres_crdt still allows the sqlite style placeholders

https://github.com/JanezStupar/postgres_crdt/tree/feature/ver4

@cachapa
Copy link
Owner

cachapa commented Oct 14, 2025

Thanks.

FYI I'm looking at the strategy used by PowerSync and there are a few concepts that I like, mainly that it run in parallel to the database monitoring for changes and performing conflict resolution on top.

This has the benefit of being able to use standard tooling or non-CRDT-aware clients to make changes to the database without breaking the HLC clock, or forcing tables to have modified and is_deleted columns.

I do have a few concerns about their implementation, mainly in terms of performance and that it seems to delegate server-side conflict resolution to your backend API if I'm interpreting this diagram properly:
image

If we use a similar approach, we might greatly simplify our implementation by the fact that we would no longer need to build wrappers around every single SQL command in existence.

@JanezStupar
Copy link
Contributor Author

JanezStupar commented Oct 14, 2025 via email

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.

2 participants