Skip to content
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

Add an 'operations.uuid' column, generated and virtual #449

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

djmitche
Copy link
Collaborator

@djmitche djmitche commented Sep 2, 2024

This column is not stored separately, but allows indexing operations by uuid without further modification or redundant storage. DB's have some cool features!

This is part of ongoing work toward #373. You can see the whole branch in main...djmitche:issue373.

This column is not stored separately, but allows indexing operations by
uuid without further modification or redundant storage.
@ryneeverett
Copy link
Collaborator

I'm not sure how exactly this fits in the latest strategy you've described in #373 but it does suggest the possibility of passing references to "losing" operations rather than values. The duplication of operations -- which basically have an unbounded size -- worries me.

@djmitche
Copy link
Collaborator Author

djmitche commented Sep 2, 2024

I'm not sure what you mean by "duplication of operations". The discussion in #373 does talk about values being duplicated, in order to indicate which operation "won" while also including "losing" operations. But that's not particularly relevant to this PR, which just allows indexing operations by task uuid.

I'm also confused by references vs. values -- that makes sense in Rust terms, but none of that is happening in this PR. And I think SQL is entirely pass-by-value, although I've not thought about that too hard.

If you meant "referencing" an operation by some unique ID, I'm not sure how we would establish that unique ID across replicas, and also ensure that the referent was always present on a replica where the reference was present.

@djmitche djmitche merged commit 427ad78 into GothenburgBitFactory:main Sep 2, 2024
13 checks passed
@ryneeverett
Copy link
Collaborator

If you meant "referencing" an operation by some unique ID, I'm not sure how we would establish that unique ID across replicas, and also ensure that the referent was always present on a replica where the reference was present.

Yeah, that is what I meant. Sorry I was using rust terminology analogously, that was cruel.

I already had in mind that a modification of the sync protocol employing operation uuid's might escape some of the obstacles we were discussing, including duplication and the invariant that the replica state is the result of all operations by separating the data of operations from the concept of "all operations". "All operations", with respect to the invariant, could be an array of uuids. And with global uuid's, duplication could probably be eliminated and transmission could be limited to the uuid rather than the whole operation. I'm just spitballing a a significant change here though and hoping you have a more elegant solution.

@ryneeverett
Copy link
Collaborator

Ok, so after reviewing #450 I now see that I had missed that these uuid's don't belong to the operation itself but the task it operates on so my suggestion was irrelevant to this PR.

@djmitche
Copy link
Collaborator Author

djmitche commented Sep 3, 2024

Ah, I see!

djmitche added a commit to djmitche/taskchampion that referenced this pull request Oct 12, 2024
djmitche added a commit that referenced this pull request Oct 13, 2024
* Revert "Temporarily make get_task_operations non-public (#464)"

This reverts commit 8f3914b.

* Revert "Additional tests for Replica methods (#459)"

This reverts commit a5f116f.

* Revert "Apply the same tests to all storage backends (#453)"

This reverts commit c7c2cde.

* Revert "Replace `Storage::set_operations` with `sync_complete` and `remove_operation`.` (#451)"

This reverts commit d7a7d96.

* Revert "Add `get_task_operations` (#450)"

This reverts commit 8bfbb9d.

* Revert "Add an 'operations.uuid' column, generated and virtual (#449)"

This reverts commit 427ad78.

* Revert "Add a test for upgrading from 0.7.0 DBs (#448)"

This reverts commit fb84873.
djmitche added a commit to djmitche/taskchampion that referenced this pull request Oct 24, 2024
…Factory#449)

This column is not stored separately, but allows indexing operations by
uuid without further modification or redundant storage.
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