-
Notifications
You must be signed in to change notification settings - Fork 31
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: add unit tests to Row #77
Conversation
bf8753f
to
5b5dbc6
Compare
spark/sql/types/row.go
Outdated
@@ -46,7 +59,7 @@ func (r *rowImpl) Len() int { | |||
} | |||
|
|||
func (r *rowImpl) FieldNames() []string { | |||
names := make([]string, len(r.offsets)) | |||
var names []string |
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.
can you fix this line to pickup the latest master changes that pre-allocates please?
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 a lot, that is the best solution, I was confused how to do it 🙏🏽 🙏🏽 🙏🏽 🙏🏽 🙏🏽
fixing ...
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.
done 👍🏽
Signed-off-by: Mustafa Elbehery <melbeher@redhat.com>
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.
merging to master
Thanks for your contribution |
What changes were proposed in this pull request?
#75 shows that
Row
type has a bug.I added tests to the
Row
types.Why are the changes needed?
To guarantee bug free implementation of
Row
type.Does this PR introduce any user-facing change?
No
How was this patch tested?
The PR adds unit-tests for the
Row
type.fixes #76
Signed-off-by: Mustafa Elbehery melbeher@redhat.com