[vibhatha]

Rationale for this change

TypeLayout contains functions to get the buffer count for a particular type and get the basic layout.
This PR represent the solution for #40934 and #40935. The solutio has been merged since there is an overlapping content when testing.

What changes are included in this PR?

This feature is implemented using only ArrowType. But for variadic buffer containing types, this approach is not feasible. So this PR introduces a new API which considers the vector itself along with the type. And this PR deprecates the older API which only used the ArrowType

Are these changes tested?

Yes

Are there any user-facing changes?

Yes, the older API is deprecated and a new API has been introduced.

This PR includes breaking changes to public APIs.

The functions TypeLayout.getTypeBufferCount and TypeLayout.getTypeLayout have been updated such that older API depcreated while the new overloaded (same function) becomes the proposed new API.

• GitHub Issue: [Java] TypeLayout enhancement to support StringView #40934

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - Contributing Overview

[vibhatha]

@lidavidm could you please take a look? I am not sure the best practices for deprecation. Should we also add docs? I mean not only Java docs, but also documentation (rst)?

[lidavidm]

Does this make sense? The vector is possibly uninitialized right now. You need to use the IPC metadata to figure out the buffer count here, not the Java vector.

[vibhatha]

Sorry, @lidavidm , I have a knowledge gap here. In this particular method we have access to the iterator with buffers. Would it be ideal if we iterate until null? I know this gives a non-initialized list. Is there an alternative?

[lidavidm]

That means we're just blindly trusting the input.

I recommend reviewing the spec again... https://github.com/apache/arrow/blob/main/format/Message.fbs#L117

[vibhatha]

Oh I see. So it seems we should update the RecordBatch to include variadicBufferCounts then we can use that information in such cases where the vector has not been initialized. That's has not yet been integrated. So probably that needs to be done here as well.

[vibhatha]

@lidavidm When we add this attribute to the RecordBatch message, should we populate non View-type vectors with 0 values or just leave it as it is but customize to pass variadicBufferCounts for View-type vectors?

[lidavidm]

What would you put besides 0?

[lidavidm]

I think it needs to be part of RecordBatch

[vibhatha]

Yes of course. I added it and I am getting some memory leaks from non variable width vectors. Need to check it.that's where I was adding the non zero variadicBufferCount views and zero for others.

[vibhatha]

That's why I was thinking whether to not populate that field for non-views.

[lidavidm]

Ditto here.

[vibhatha]

@lidavidm I think the scope of this PR was broadened than I anticipated. This needs more test coverage for RecordBatch spec change and its usage across the codebase. I am going to convert this PR into a draft.

I will probably need to create a new issue as a parent issue to host the sub-tasks done here. So I am thinking about creating a new PR once all angels are covered for each issue addressed. I am planning this because, it is kind of hard to make independent PRs to get the expected functionality and rather make them in a single one. Hope this is okay with you?

[lidavidm]

sure

[vibhatha]

Closing this PR in favor of #41732