[query] Offset function #1561
[query] Offset function #1561
Conversation
|
General question on this: for most operations, we apply them eagerly--that is, we iterate through the block and create a new block. What makes it such that |
src/query/executor/transform/exec.go
Outdated
|
|
||
| defer nextBlock.Close() | ||
| return controller.Process(queryCtx, nextBlock) | ||
| // ProcessWrapperBlock works similarly to ProcessSimpleBlock, but does not close |
There was a problem hiding this comment.
This is probably debatable, but maybe best to just not use the helper in this case? There are already functions which don't (for basically the same reason--they need more control over the block lifecycle). Especially because we comment pretty explicitly down below; might be more clear to just manage the lifecycle directly there.
There was a problem hiding this comment.
Sounds good, did it this way mostly because I didn't realize that we already had a couple of blocks that don't use it, so happy to revert these changes.
That's true for most functions, since they generally collapse the input space during execution (temporal functions excluded unfortunately), but we have some functions which we can apply lazily as a simple transform, which can then defer decoding until an aggregation function is ready to collapse the results. We do have some code in place for lazily applying functions, but it's pretty awkward to use and probably needs an update to look more like this model (i.e. pass down a function into |
|
Nice, this looks a ton better than the other alternative! |
Codecov Report
@@ Coverage Diff @@
## master #1561 +/- ##
========================================
+ Coverage 71.8% 71.8% +<.1%
========================================
Files 946 949 +3
Lines 78019 78206 +187
========================================
+ Hits 56027 56165 +138
- Misses 18348 18394 +46
- Partials 3644 3647 +3
Continue to review full report at Codecov.
|
Updated offset to create a downstream processing function rather than fixing in iterators