Views from host #3548
Open
Views from host #3548
+1,404
−342
Conversation
This file contains hidden or 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
# Description of Changes Creates a multi-column index on the `sender` and `arg_id` columns of a view's backing table so that we can quickly retrieve a view's result set for each caller. <!-- Please describe your change, mention any related tickets, and so on here. --> # API and ABI breaking changes <!-- If this is an API or ABI breaking change, please apply the corresponding GitHub label. --> None This is an internal index. No module bindings are generated for it. Uniqueness of the index name follows from the uniqueness of the table name since views live in the table namespace. # Expected complexity level and risk <!-- How complicated do you think these changes are? Grade on a scale from 1 to 5, where 1 is a trivial change, and 5 is a deep-reaching and complex change. This complexity rating applies not only to the complexity apparent in the diff, but also to its interactions with existing and future code. If you answered more than a 2, explain what is complex about the PR, and what other components it interacts with in potentially concerning ways. --> 1 # Testing <!-- Describe any testing you've done, and any testing you'd like your reviewers to do, so that you're confident that all the changes work as expected! --> Performance change
Shubham8287
force-pushed
the
shub/view-from-host
branch
2 times, most recently
from
0f58cb8 to
1abf255
Compare
Shubham8287
force-pushed
the
shub/view-from-host
branch
from
1abf255 to
ce0d15a
Compare
Shubham8287
commented
Oct 31, 2025
# Description of Changes Addresses - #3487 (comment). Can be reviewed commit wise. 1. Provide `ModuleSubscriptions` a interface to invoke `ModuleHost::call_view`. 2. Handle `st_view_client table` insertion. 3. Make views available form sql.
Shubham8287
force-pushed
the
shub/view-from-host
branch
2 times, most recently
from
4a475a2 to
c2f43a1
Compare
Shubham8287
force-pushed
the
shub/view-from-host
branch
3 times, most recently
from
9ed65e4 to
fa156e1
Compare
Shubham8287
force-pushed
the
shub/view-from-host
branch
from
fa156e1 to
131fdba
Compare
Shubham8287
force-pushed
the
shub/view-from-host
branch
from
564a081 to
655ab83
Compare
| self.instance_env.start_view(ts, view); | ||
| } | ||
| } | ||
| self.instance_env.start_funcall(ts); |
There was a problem hiding this comment.
I think this call is redundant now.
Comment on lines
+422
to
+424
| // Signal that this reducer call is finished. This gets us the timings | ||
| // associated to our reducer call, and clears all of the instance state | ||
| // associated to the call. |
There was a problem hiding this comment.
Copy errors
Suggested change
| // Signal that this reducer call is finished. This gets us the timings | |
| // associated to our reducer call, and clears all of the instance state | |
| // associated to the call. | |
| // Signal that this view call is finished. This gets us the timings | |
| // associated to our view call, and clears all of the instance state | |
| // associated to the call. |
Comment on lines
+465
to
+467
| // Signal that this reducer call is finished. This gets us the timings | ||
| // associated to our reducer call, and clears all of the instance state | ||
| // associated to the call. |
There was a problem hiding this comment.
Same copy errors
Suggested change
| // Signal that this reducer call is finished. This gets us the timings | |
| // associated to our reducer call, and clears all of the instance state | |
| // associated to the call. | |
| // Signal that this view call is finished. This gets us the timings | |
| // associated to our view call, and clears all of the instance state | |
| // associated to the call. |
Comment on lines
-331
to
+348
| F: FnOnce(&RelationalDB) -> T + Send + 'static, | ||
| F: FnOnce(Arc<RelationalDB>) -> Fut + Send + 'static, | ||
| Fut: std::future::Future<Output = T> + Send + 'static, |
There was a problem hiding this comment.
Why is this change required? It might be a good place to add a comment for future readers.
Comment on lines
+712
to
+714
| // Merge read sets from the `MutTxId` into the `CommittedState`. | ||
| // It's important that this happens after applying the changes to `tx_data`, | ||
| // so that we can pass updated set of table ids. |
There was a problem hiding this comment.
I would also add here that tx_data already contains the inserts and deletes for the view tables
| fn merge_read_sets(&mut self, read_sets: ViewReadSets) { | ||
| for (view_id, read_set) in read_sets { | ||
| self.merge_read_set(view_id, read_set); | ||
| fn merge_read_sets(&mut self, read_sets: ViewReadSets, tables: impl IntoIterator<Item = TableId>) { |
There was a problem hiding this comment.
Can you rename the tables param to dropped_tables or something similar?
| Ok((tx, commit)) | ||
| } | ||
|
|
||
| /// Checks whether a memoized view exists for the given view name, arguments, and sender identity. |
There was a problem hiding this comment.
Suggested change
| /// Checks whether a memoized view exists for the given view name, arguments, and sender identity. | |
| /// Checks whether a materialized view exists for the given view name, arguments, and sender identity. |
| let mut metrics = ExecutionMetrics::default(); | ||
|
|
||
| for (view_name, args) in stmt.views() { | ||
| let (is_memoized, args) = tx |
There was a problem hiding this comment.
nit: is_materialized
| execution_duration: result.timings.total_duration, | ||
| }; | ||
|
|
||
| (res, trapped) |
There was a problem hiding this comment.
So we don't have to explicitly handle traps? They're handled a layer above?
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
Host implementation to invoke
call_viewmethod.I also covers:
MutTxId::is_materializedto check if existing view exisits and updated.API and ABI breaking changes
NA
Testing