Skip to content

Conversation

@StevenMPhillips
Copy link
Contributor

…Union vectors

Includes a fix for splitAndTransfer in BitVector for the case when split length is 0

…Union vectors

Includes a fix for splitAndTransfer in BitVector for the case when split length is 0
@xhochy
Copy link
Member

xhochy commented Jul 17, 2017

@StevenMPhillips Can you give a bit more context on this change so that I'm able to review it?

@wesm
Copy link
Member

wesm commented Jul 17, 2017

Needs a unit test also

@jacques-n
Copy link
Contributor

@siddharthteotia, can you take over this pull request and add some unit tests?

The goal of this PR was to update some vectors so they were more efficient at splitting vectors into subvectors. Many vectors are already efficient, minimizing manipulation of data. However, the ListVector and BitVector were doing far more work than necessary. In Dremio we use splitAndTransfer when unrolling lists. Ensuring that these apis are efficient substantially reduces CPU overhead during this operation. See the code using it here: https://github.com/dremio/dremio-oss/blob/master/sabot/kernel/src/main/java/com/dremio/sabot/op/flatten/FlattenTemplate.java#L205.

Probably makes sense to move our Dremio code for this class over to Arrow as well:

https://github.com/dremio/dremio-oss/blob/master/sabot/kernel/src/test/java/com/dremio/exec/vector/TestSplitAndTransfer.java

@siddharthteotia
Copy link
Contributor

siddharthteotia commented Jul 20, 2017

@jacques-n
Sure. I will add the unit tests.

@siddharthteotia
Copy link
Contributor

I have opened a new PR -- #901

@wesm
Copy link
Member

wesm commented Jul 29, 2017

This one can be closed

@wesm
Copy link
Member

wesm commented Aug 4, 2017

Can this be closed?

@jacques-n
Copy link
Contributor

@StevenMPhillips, please close.

@wesm
Copy link
Member

wesm commented Sep 19, 2017

@StevenMPhillips can you close?

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.

5 participants