-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
colexechash: introduce a type alias for keyID #75595
Conversation
955c995
to
a5887ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this!
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @michae2 and @yuzefovich)
pkg/sql/colexec/colexechash/hashtable.go, line 73 at r1 (raw file):
// // keyID of 0 is reserved to indicate the end of the hash chain. type keyID uint64
Ah, crap, I misled you. I was wrong, this should probably be a "type alias declaration" not a "type definition" (TIL they are different in go!). In other words, this should probably be:
type keyID = uint64
And by adding that equals sign, we should be able to use []uint64
as []keyID
and vice-versa (even without explicit casts). Then you won't need the unsafe.Pointer
stuff and in general that will make life much easier. Sorry about that!
Code quote:
type keyID uint64
Release note: None
a5887ce
to
ecdcc4b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @michae2)
pkg/sql/colexec/colexechash/hashtable.go, line 73 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
Ah, crap, I misled you. I was wrong, this should probably be a "type alias declaration" not a "type definition" (TIL they are different in go!). In other words, this should probably be:
type keyID = uint64
And by adding that equals sign, we should be able to use
[]uint64
as[]keyID
and vice-versa (even without explicit casts). Then you won't need theunsafe.Pointer
stuff and in general that will make life much easier. Sorry about that!
Oops, this makes sense, I don't know why I didn't think of this myself right away, I guess I wasn't consciously aware of these differences too. PTAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @michae2)
TFTR! bors r+ |
Build succeeded: |
Release note: None