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

GH-48: Implement VectorAppender for BaseVariableWidthViewVector #454

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ViggoC
Copy link
Contributor

@ViggoC ViggoC commented Dec 10, 2024

Fixes #48.

@ViggoC ViggoC force-pushed the viggo/string-view-appender branch from bfa1bcf to 5470deb Compare December 19, 2024 17:20
@ViggoC ViggoC marked this pull request as ready for review December 19, 2024 17:21
Copy link
Member

Choose a reason for hiding this comment

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

Does this test the case of multiple data buffers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean ViewVarCharVector.dataBuffers.size() > 1. I'm afraid not. I'll check it, and add a case if it's not covered.

VectorAppender appender = new VectorAppender(target);
delta.accept(appender, null);

assertEquals(2, target.getDataBuffers().size());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lidavidm ViewVarCharVector.dataBuffers.size() > 1 is already covered in this test case.

Copy link
Member

Choose a reason for hiding this comment

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

But what about the cases where target and delta have more than one data buffer before appending?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's covered in e918540.

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.

[Java] Implement VectorAppender for BaseVariableWidthViewVector
2 participants