Fix hashjoin runtime issue #106
Merged
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
Author
|
cc @zhejiangxiaomai , |
Author
|
cc @rui-mo |
zhejiangxiaomai
approved these changes
Jan 4, 2023
rui-mo
reviewed
Jan 4, 2023
| } | ||
|
|
||
| // Identify the non-key build side columns and make a decoder for each. | ||
| const auto numDependents = outputType->size() - numKeys; |
Collaborator
There was a problem hiding this comment.
In the test case you mentioned, what is the number of numDependents? I guess the reserve function fails only when the reserved value is non-positive, am I correct?
Author
There was a problem hiding this comment.
@rui-mo，
If the value < 0, will trigger exception.
terminate called after throwing an instance of 'std::length_error'
what(): vector::reserve
Aborted
Collaborator
There was a problem hiding this comment.
I guess a better solution would be to check whether the reserved value is greater than 0. If yes, the reserve functions can be called.
Author
There was a problem hiding this comment.
If-Else branch is not better for CPU performance.
And here, reserve will not bring benefit because output schema field is limited.
Collaborator
There was a problem hiding this comment.
To reserve before use the vector is a common code style in Velox, and they have gave me some comments on that before. I think we'd better follow that. As for the performance, I think if else would not affect the performance too much here.
Collaborator
There was a problem hiding this comment.
Advantages to use reserve: https://www.geeksforgeeks.org/using-stdvectorreserve-whenever-possible/.
Collaborator
|
Maybe we need to discuss this issue with Velox, and use the upstreamed fix. Merge this for now. |
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Jan 5, 2023
zhejiangxiaomai
pushed a commit
that referenced
this pull request
Jan 11, 2023
zhejiangxiaomai
pushed a commit
that referenced
this pull request
Jan 11, 2023
zhejiangxiaomai
pushed a commit
that referenced
this pull request
Jan 31, 2023
zhejiangxiaomai
pushed a commit
that referenced
this pull request
Feb 22, 2023
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Feb 27, 2023
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Mar 6, 2023
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Mar 27, 2023
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Mar 29, 2023
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Mar 29, 2023
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Apr 14, 2023
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Apr 17, 2023
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Apr 19, 2023
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Apr 20, 2023
zhejiangxiaomai
added a commit
to zhejiangxiaomai/velox
that referenced
this pull request
May 31, 2023
relative pr: Fix hashjoin runtime issue oap-project#106 INVALID_STATE on HashJoin when spill is turned on oap-project#154 SIGABRT on DecimalAvgAggregate<UnscaleLongDecimal, UnscaleShortDecimal> when spilling is engaged oap-project#236 Support kPreceeding & kFollowing for window range frame type oap-project#287
zhejiangxiaomai
added a commit
to zhejiangxiaomai/velox
that referenced
this pull request
May 31, 2023
relative pr: Fix hashjoin runtime issue oap-project#106 INVALID_STATE on HashJoin when spill is turned on oap-project#154 SIGABRT on DecimalAvgAggregate<UnscaleLongDecimal, UnscaleShortDecimal> when spilling is engaged oap-project#236 Support kPreceeding & kFollowing for window range frame type oap-project#287
zhejiangxiaomai
added a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Jul 3, 2023
relative pr: Fix hashjoin runtime issue oap-project#106 INVALID_STATE on HashJoin when spill is turned on oap-project#154 SIGABRT on DecimalAvgAggregate<UnscaleLongDecimal, UnscaleShortDecimal> when spilling is engaged oap-project#236 Support kPreceeding & kFollowing for window range frame type oap-project#287
zhejiangxiaomai
added a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Jul 4, 2023
relative pr: Fix hashjoin runtime issue oap-project#106 INVALID_STATE on HashJoin when spill is turned on oap-project#154 SIGABRT on DecimalAvgAggregate<UnscaleLongDecimal, UnscaleShortDecimal> when spilling is engaged oap-project#236 Support kPreceeding & kFollowing for window range frame type oap-project#287
zhejiangxiaomai
added a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Jul 11, 2023
relative pr: Fix hashjoin runtime issue oap-project#106 INVALID_STATE on HashJoin when spill is turned on oap-project#154 SIGABRT on DecimalAvgAggregate<UnscaleLongDecimal, UnscaleShortDecimal> when spilling is engaged oap-project#236 Support kPreceeding & kFollowing for window range frame type oap-project#287
zhejiangxiaomai
added a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Jul 12, 2023
relative pr: Fix hashjoin runtime issue oap-project#106 INVALID_STATE on HashJoin when spill is turned on oap-project#154 SIGABRT on DecimalAvgAggregate<UnscaleLongDecimal, UnscaleShortDecimal> when spilling is engaged oap-project#236 Support kPreceeding & kFollowing for window range frame type oap-project#287
zhejiangxiaomai
added a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Jul 12, 2023
relative pr: Fix hashjoin runtime issue oap-project#106 INVALID_STATE on HashJoin when spill is turned on oap-project#154 SIGABRT on DecimalAvgAggregate<UnscaleLongDecimal, UnscaleShortDecimal> when spilling is engaged oap-project#236 Support kPreceeding & kFollowing for window range frame type oap-project#287
zhejiangxiaomai
added a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Jul 17, 2023
relative pr: Fix hashjoin runtime issue oap-project#106 INVALID_STATE on HashJoin when spill is turned on oap-project#154 SIGABRT on DecimalAvgAggregate<UnscaleLongDecimal, UnscaleShortDecimal> when spilling is engaged oap-project#236 Support kPreceeding & kFollowing for window range frame type oap-project#287
marin-ma
added a commit
to marin-ma/velox-oap
that referenced
this pull request
Dec 15, 2023
* Add doc for arrow backend * README.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Here, reserve action doesn't bring benefit.
Instead, it will cause issues when outputType->size() < numKeys. Such as: apache/incubator-gluten#790.
So just remove them.