[multistage][bugfix] fix operator eos pull #11970
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
Codecov Report
@@ Coverage Diff @@
## master #11970 +/- ##
=============================================
- Coverage 61.43% 27.60% -33.84%
+ Complexity 1135 201 -934
=============================================
Files 2385 2385
Lines 129150 129179 +29
Branches 19994 19998 +4
=============================================
- Hits 79347 35657 -43690
- Misses 44049 90837 +46788
+ Partials 5754 2685 -3069
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1165 files with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
Jackie-Jiang
reviewed
Nov 8, 2023
| @@ -132,25 +132,22 @@ public String toExplainString() { | |||
| @Override | |||
| protected TransferableBlock getNextBlock() { | |||
| try { | |||
There was a problem hiding this comment.
Do we always need to do this try-catch? Consider moving it into base class?
There was a problem hiding this comment.
yes i can move it to base
| // construct a SET with all the right side rows. | ||
| constructRightBlockSet(); | ||
| } | ||
| if (_upstreamErrorBlock != null) { |
There was a problem hiding this comment.
We probably don't need to save this as member variable. Also, the error from right table is not gathered
There was a problem hiding this comment.
yeah we should've :-P the issue here is similar to JOIN, fixed
Jackie-Jiang
approved these changes
Nov 9, 2023
| @@ -70,7 +79,7 @@ public String getOperatorId() { | |||
| } | |||
|
|
|||
| // Make it protected because we should always call nextBlock() | |||
| protected abstract TransferableBlock getNextBlock(); | |||
| protected abstract TransferableBlock getNextBlock() throws Exception; | |||
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.
currently agg, window, transform, fitler operator can potentially pull more than once against input for EOS.
This PR changes the behavior.
this guarantees that no more than one EOS block will be pulled; tests are covered by existing resource queries tests