-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
lookaside for primary keys #344
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tantaman
force-pushed
the
compact-pk
branch
2 times, most recently
from
September 14, 2023 16:16
568d43a
to
9d49a28
Compare
We can do this since we're moving all user controlled columns out of the clock tables and into `[table_name]__crsql_pks` clock tables currently still use user provided columns as the primary keys so we can test this change on its own before making the sweeping change of using the lookaside table for the clock table primary key
We may abandon triggers all together in the near future and either: - use the preupdate hook - the update hook - strip trigger bodies to only invoke a single function or virtual table
The problem is trigger constraints. We need the key from the lookup table. We can't set a variable in a trigger so I was going to do: ```sql SELECT set_value(SELECT key FROM lookaside WHERE ...); ``` and then use `get_value` later. Problem is, set value may not get set. So we need to conditionally insert and set the value again after the insert. Further problem is we can't do that in a trigger. So the new plan... Stop using triggers, mostly. Trigger bodies will now be: ```sql VALUES (process_insert/update/delete('table', 'column', value, pks...)); ``` and the `process_*` function(s) will take care of it. This mean we should re-work our statement caching to: 1. No longer require an indirect lookup via b-tree 2. Be stored on `table_info` structs 3. Be re-computed after an alter of that specific `CRR` These statements should probably be lazily computed to reduce extension load time? Since serverless (ugh) environments may re-load the extension between every request.
tantaman
force-pushed
the
compact-pk
branch
from
September 19, 2023 19:29
2050df6
to
b6d608e
Compare
Will this be a breaking change or is there an easy non-breaking upgrade path? It's fine if not, but I'll need to know since we released |
Definitely breaking but we could put together a migration script that fills
in the lookaside tables as an upgrade path.
…On Fri, Sep 22, 2023 at 8:32 AM Jerome Gravel-Niquet < ***@***.***> wrote:
Will this be a breaking change or is there an *easy* non-breaking upgrade
path? It's fine if not, but I'll need to know since we released 0.1.0 of
Corrosion and I want to upgrade to this as soon as possible :)
—
Reply to this email directly, view it on GitHub
<#344 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHWK23M3TFXEQYHSSK7WGLX3WHNXANCNFSM6AAAAAA4N34SSA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
This was referenced Sep 26, 2023
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Primary keys for CRRs are generally going to be UUIDs or some other large-ish thing.
Moving them out into a lookaside lets us keep a variable length encoded int in the clock tables instead of the full primary key. This is useful since clock tables keep an entry per cell rather than a single entry per row.
In tables with < 65k entries this save 14 bytes. In tables not exceeding 4 billion entries, saves us 12 bytes.
(aside: we can use 64 bit ints in many cases for primary keys in cases where we can segment the population similar to https://mariadb.com/kb/en/uuid_short/)