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

[columnar] HYD-761 Set maximum column size #222

Merged
merged 2 commits into from
Jan 11, 2024
Merged

[columnar] HYD-761 Set maximum column size #222

merged 2 commits into from
Jan 11, 2024

Conversation

mkaruza
Copy link
Contributor

@mkaruza mkaruza commented Jan 10, 2024

Metadata table is on heap. If we try to construct metadata row bigger than 1GB insert will fail at that point. Limit maximum column size to be 256MB.

* When we are building chunk for serialization if serialzation memory
  is higher than 256MB, memcpy_s function (safeclib) will not copy
  anything (there is memory size check).
  Fixed by using memcpy. We have allocated already memory with required
  size so it should be safe to copy it.
* Metadata table is on heap. If we try to construct metadata row bigger
  than 1GB insert will fail at that point. Limit maximum column size to
  be 256MB.
Copy link
Contributor

@JerrySievert JerrySievert left a comment

Choose a reason for hiding this comment

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

code changes a-ok, but we should wait on @wuputah's agreement before merge.

@wuputah wuputah self-requested a review January 11, 2024 22:41
Copy link
Member

@wuputah wuputah left a comment

Choose a reason for hiding this comment

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

I'm OK with it.

@wuputah wuputah merged commit 4de97ca into main Jan 11, 2024
@wuputah wuputah deleted the HYD-761 branch January 11, 2024 22:43
@wuputah wuputah mentioned this pull request Jan 26, 2024
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.

3 participants