Nuke to_mem_table_with_op_type
#990
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
Rather than annotating rows with `__op_type` during `eval_incr` of selects, partition the rows before evaluation, then merge after.
Centril
reviewed
Mar 18, 2024
| row.elements.remove(pos_op_type).into_u8().unwrap_or_else(|_| { | ||
| panic!("Failed to extract `{OP_TYPE_FIELD_NAME}` from `{}`", header.table_name) | ||
| }); | ||
| let row = row_ref.into_product_value(); | ||
| into.push(TableOp::new(op_type, row)); |
There was a problem hiding this comment.
Perfect; next step (I'll fix it): make TableOp hold a RelValue
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com> Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
…bs/spacetimedb into phoebe/incr-select-partition
joshua-spacetime
approved these changes
Mar 18, 2024
bfops
approved these changes
Mar 18, 2024
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
Rather than annotating rows with
__op_typeduringeval_incrof selects, partition the rows before evaluation, then merge after.API and ABI breaking changes
N/a
Expected complexity level and risk
2 - a purely interior change to
eval_incrofSELECTqueries, but if incorrect could lead to subtle weird behaviors. Probably worth a bot test before merging.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!
incr-selectbenchmark, saw: