Rework RLS macro for amended syntax #2105
Merged
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
Out-of-band discussion resulted in a different syntax for registering RLS rules in module code. This PR implements that new syntax in Rust modules. C# modules, which did not have an implementation prior, are unaffected. The new macro's implementation is significantly simpler, with all the interesting work done by "runtime" code rather than in the macro. One effect of this is that SQL syntax errors are no longer detected at compile time. Instead, they will only be checked at `spacetime publish` time, when the filter query is processed by the SpacetimeDB host. I consider this reasonable, as even in the previous design, most "interesting" errors (e.g. name resolution, type checking) could only be detected at publish time anyways. The new macro could perform SQL syntax checking in the same way as the old one, but I consider the benefits of this to be outweighed by the added complexity in our macro code it would require. This commit also applies `#[doc(hidden)]` to everything RLS-related, with TODO comments to make them visible once RLS is fully implemented.
gefjon
requested review from
mamcx,
RReverser,
coolreader18 and
joshua-spacetime
Prior to this commit, `DESCRIBERS` was a vec of function pointers. This meant that each `DESCRIBER` needed to be its own function, rather than a closure over the thing being described. With this commit, `DESCRIBERS` is amended to hold boxed closures. This allows `register_row_level_security` to operate without a type parameter.
joshua-spacetime
approved these changes
Jan 14, 2025
Co-authored-by: joshua-spacetime <[email protected]> Signed-off-by: Phoebe Goldman <[email protected]>
Co-authored-by: joshua-spacetime <[email protected]> Signed-off-by: Phoebe Goldman <[email protected]>
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.
Description of Changes
Out-of-band discussion resulted in a different syntax for registering RLS rules in module code. This PR implements that new syntax in Rust modules. C# modules, which did not have an implementation prior, are unaffected.
The new macro's implementation is significantly simpler, with all the interesting work done by "runtime" code rather than in the macro. One effect of this is that SQL syntax errors are no longer detected at compile time. Instead, they will only be checked at
spacetime publishtime, when the filter query is processed by the SpacetimeDB host. I consider this reasonable, as even in the previous design, most "interesting" errors (e.g. name resolution, type checking) could only be detected at publish time anyways. The new macro could perform SQL syntax checking in the same way as the old one, but I consider the benefits of this to be outweighed by the added complexity in our macro code it would require.This commit also applies
#[doc(hidden)]to everything RLS-related, with TODO comments to make them visible once RLS is fully implemented.API and ABI breaking changes
No, in the sense that RLS is not actually implemented, and so no one could possibly have been depending on it.
Expected complexity level and risk
1: new macro is much simpler; internals are unchanged.
Testing