Skip to content

Rework C++ truncation for arrow #2387

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

Draft
wants to merge 1 commit into
base: read_as_arrow
Choose a base branch
from
Draft

Conversation

IvoDD
Copy link
Collaborator

@IvoDD IvoDD commented Jun 5, 2025

Monday ref: 9320469407

Arrow requires truncation to happen in C++ layer.
Previous logic had many issues:

  • Wrong truncated range calculation
  • Wrong offsets used when first row offset != 0
  • Wrong row range calculation

This PR fixes the issues by rewriting a lot of the column truncation logic and adds more tests.

Reference Issues/PRs

What does this implement or fix?

Any other comments?

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

@IvoDD IvoDD force-pushed the read_as_arrow branch 2 times, most recently from 65d4e59 to bef748c Compare June 6, 2025 06:40
@IvoDD IvoDD force-pushed the rework-cpp-truncation branch from 6f33a19 to 2ecfee1 Compare June 6, 2025 14:56
Arrow requires truncation to happen in C++ layer.
Previous logic had many issues:
- Wrong truncated range calculation
- Wrong offsets used when first row offset != 0
- Wrong row range calculation

This PR fixes the issues by rewriting a lot of the column truncation
logic and adds more tests.
@IvoDD IvoDD force-pushed the rework-cpp-truncation branch from 2ecfee1 to d28b7a5 Compare June 6, 2025 15:06
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.

1 participant