Created wrappers around Internal IDs for consistency and added an ID dispatch registry #1107
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.
Currently, an
Internal
ID requires one to know the exact format for constructing an ID of a specific type. For example,Internal
table IDs have the following format:Not only would you need to know that this is the format, but when retrieving values, you'd have to make sure that you accessed index 0 for the schema, and index 1 for the table. This opens up a potential for hard-to-find bugs. Wrappers help with this. Internally, the wrappers will call index 0 or 1, but one now just calls
SchemaName()
orTableName()
. In addition, they don't need to know the format, callingNewInternalTable
gives you the parameters that you need.Additionally, this adds a registry where schema elements may register callbacks to respond to ID changes. For example, rather than having a table column explicitly handle all locations that depend on the column, it instead broadcasts that the column has changed, and all locations that have registered for that broadcast will modify themselves. This helps with scaling, as Postgres is far more inter-dependent than MySQL/Dolt, and reduces the knowledge burden required by developers.
Currently it's only used by sequences in this PR, and that's primarily due to most actions being handed off to GMS and Doltgres having no record of them. For example, deleting a column is completely handled by GMS, but Doltgres needs to broadcast that the column was deleted. For now, the idea is to wrap all such nodes in a Doltgres node at the end of the analyzer step (in
ReplaceNode
), but I'm open to another idea since it essentially means we're making a wrapper for the majority of GMS nodes. That's a separate issue from this PR though.