Add args column to view backing tables#5300
Conversation
kistz
left a comment
There was a problem hiding this comment.
This looks very nice!
Hashing is correctly encapsulated and all call sites use the appropriate methods as far as i saw :>
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub struct ParamSlot(pub u16); | ||
|
|
||
| pub const PARAM_SENDER: ParamSlot = ParamSlot(0); | ||
| pub const PARAM_VIEW_ARG_HASH: ParamSlot = ParamSlot(1); |
There was a problem hiding this comment.
This seems like a good approach.
I think it would have great value if we could add ConnectionId to a view in the future because right now this is one part where you cannot use views. (and now its also more reasonable because sender is not super special anymore)
I suggest offsetting PARAM_VIEW_ARG_HASH by 1 or two (for safety in case another thing comes up).
That we could potentially expose the connection id in the future without breaking stuff.
There was a problem hiding this comment.
We will be definitely adding ConnectionId in the future for ease of use, although something to note is that once we have general parameters, your view will be able to take ConnectionId as an explicit argument.
As far as the offset is concerned, these slot IDs are completely internal and can be update at any point in the future without affecting api compatibility.
Description of Changes
Review #5287 first.
This patch updates view backing table schemas to have a single private column
arg_hash. Previously sender-scoped views had asendercolumn for the calling identity, however now that's been replaced with a single unifiedarg_hashcolumn that encodes the calling identity within it.When we add parameterized views, the view args will also be encoded in this hash and stored in this column.
This column exists for both anonymous and sender scoped views meaning that the backing tables for all views now have the same number of private columns - one.
This hash is now used as a runtime variable that the query engine uses to evaluate view table scans.
In order to keep the diff small, this patch does update view read sets with this new hash value. That change has a larger blast radius and will be done in the next set of changes.
API and ABI breaking changes
N/A
Expected complexity level and risk
2
Testing
Existing coverage.