feat: Pull based native execution #69
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
|
This is ported from internal patch. |
sunchao
approved these changes
Feb 21, 2024
|
Merged. Thanks. |
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.
Which issue does this PR close?
Closes #70.
Rationale for this change
Comet native execution's scan is not started from native but from JVM. Thus Comet scan is push-based instead of pull-based. Although we pull next input batches from child operator in JVM, this new input is not pulled from native but pushed from JVM side.
For an operator like Expand, one input batch can produces multiple output batches. So we cannot pull next batch directly and push into native without peeking it. We need to "peek" into native side and see if any more output batch there. If so, we take it as next output, if not, we pull next input batch and push into native side to execute on it.
If we pull next input from child operator and push it into native without peek, new input will be ignored.
Not only we cannot have consistent way to get input for native operators. The code of input/output to native execution is harder to understand because we mix push-based and pull-based processing modes. This patch tries to make native execution fully pull-based.
What changes are included in this PR?
How are these changes tested?
Passed existing tests and e2e tests.