Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix infinite loop during planning caused by PickTableLayout #17720

Merged
merged 1 commit into from
May 10, 2022

Conversation

v-jizhang
Copy link
Contributor

Replaces #16627

Co-authored-by: Brian Li librian415@gmail.com

Test plan - Added a test.

== RELEASE NOTES ==

General Changes
* Fix infinite loop during planning caused by PickTableLayout

@rohanpednekar
Copy link
Contributor

@highker you did approve this PR at #16627 but it was missing few tests.
Now @v-jizhang has added with rebase, could you please approve and merge?

Copy link
Contributor

@NikhilCollooru NikhilCollooru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For primitive types like int, please directly compare their values instead of using Objects.equals across this PR.

Direct value comparison for primitive types is much more efficient than Object.equals in Java

}
HashTables other = (HashTables) obj;
return Arrays.equals(this.hashTables, other.hashTables) &&
Objects.equals(this.expectedHashTableCount, other.expectedHashTableCount);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expectedHashTableCount is of primitive type int. We can directly compare the values as it is much more efficient than using Object.equals

this.expectedHashTableCount == other.expectedHashTableCount

return false;
}
ArrayBlock other = (ArrayBlock) obj;
return Objects.equals(this.arrayOffset, other.arrayOffset) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For primitive types like int, please directly compare their values instead of using Object.equals , here and in other files.

@v-jizhang v-jizhang requested a review from NikhilCollooru May 9, 2022 23:40
@NikhilCollooru
Copy link
Contributor

NikhilCollooru commented May 9, 2022

@v-jizhang Please squash the commits into one commit. The code changes look good.

Replaces prestodb#16627

Co-authored-by: Brian Li <librian415@gmail.com>
@v-jizhang
Copy link
Contributor Author

@v-jizhang Please squash the commits into one commit. The code changes look good.

Squashed. Thank you!

@NikhilCollooru NikhilCollooru requested a review from rschlussel May 10, 2022 16:34
@rschlussel rschlussel merged commit bd48fc3 into prestodb:master May 10, 2022
@highker highker mentioned this pull request Jul 6, 2022
7 tasks
feilong-liu added a commit to feilong-liu/presto that referenced this pull request Sep 1, 2022
Custom hashcode and equal functions are introduced for xxxBlock in PR prestodb#17720, where content in the corresponding
block rather than the object itself are used in hashcode and equal function computation. However, xxxBlockBuilder
which also extends from Block interface is left behind. In this commit, we override such functions for block builder.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants