-
Notifications
You must be signed in to change notification settings - Fork 47
[FEATURE] Add serialization/deserialization support for list of tuples, optional list, optional tuple #92
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
[FEATURE] Add serialization/deserialization support for list of tuples, optional list, optional tuple #92
Conversation
LeonhardFS
left a comment
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.
Great work here! Couple minor things, mostly doc. Else, I think there's one Python ref counting issue, else all good.
| */ | ||
| PyObject *createPyObjectFromMemory(const uint8_t *ptr, const python::Type &row_type, const uint8_t *bitmap = nullptr, int index = 0); | ||
| PyObject * | ||
| createPyObjectFromMemory(const uint8_t *ptr, const python::Type &row_type, int64_t capacity, |
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.
why int64_t? why not size_t?
| * @param objectType current object type that contains optional value | ||
| * @param ptr memory location to where the start of bitmap | ||
| * @param numElements number of elements in objectType for which bitmap is needed | ||
| * @return |
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.
return docstring?
Add serialization/deserialization support for list of tuples, list of lists, optional list, and optional tuple