DRILL-4455: Depend on Apache Arrow by StevenMPhillips · Pull Request #398 · apache/drill · GitHub
Skip to content

DRILL-4455: Depend on Apache Arrow - #398

Closed
StevenMPhillips wants to merge 5 commits into
apache:masterfrom
StevenMPhillips:arrow9
Closed

StevenMPhillips wants to merge 5 commits into
apache:masterfrom
StevenMPhillips:arrow9

Conversation

@StevenMPhillips

Copy link
Copy Markdown
Contributor

Remove the vector and memory modules
Replace most instances of TypeProtos.MajorType, TypeProtos.MinorType etc
with corresponding org.apache.arrow.types.Types class.
The code for serializing and deserializing vectors is still in the Drill code-base
and still uses the old TypeProtos classes. The class MajorTypeHelper is used
to convert between the two sets of classes.
The old load and getMetadata classes in ValueVector are now found in a corresponding
ValueVectorHelper class

@jaltekruse

Copy link
Copy Markdown
Contributor

@StevenMPhillips

Copy link
Copy Markdown
Contributor Author

I just rebased the patch and broke it up into 5 different patches.

@parthchandra

Copy link
Copy Markdown
Contributor

I have two problems with this patch (apart from it being just too big to review)

  1. Building against 0.1-SNAPSHOT of a project that is in rapid development is a courageous decision (I would rather not board a train while it is moving at full speed). The current patch also does not build as there is no artifact for Arrow 0.1-SNAPSHOT.
  2. I see that the entire memory module of Drill is gone. I don't think that drill memory management and accounting should be outsourced to Arrow. I can see needing to use ArrowBuf for value vectors but removing the entire memory module is not OK. I can see other uses of Direct/Heap memory (caching metadata for example) that should be managed under a Drill resource manager. This ties back to Mavenized query-parse subproject #1. Arrow needs to get its design figured out to allow Drill and other projects to manage their resources while providing a 'reference' implementation.
    -1 until we resolve this.

@jacques-n

Copy link
Copy Markdown
Contributor

Sounds like we need to do some more work to mature and stabilize the Arrow code. Let's pick this up again once we've made more progress on that front.

@ilooner

ilooner commented Jun 8, 2018

Copy link
Copy Markdown
Contributor

@vrozov Since you are picking up Drillbuf design work, please advise on the plan for this PR.

gparai pushed a commit to gparai/drill that referenced this pull request Oct 10, 2018
…xpressions (apache#398)

* Fixed pushing istrue and isfalse conditions. Added unit tests to test boolean conditions.
karthik-man pushed a commit to karthik-man/drill that referenced this pull request Mar 14, 2019
…xpressions (apache#398)

* Fixed pushing istrue and isfalse conditions. Added unit tests to test boolean conditions.
KazydubB pushed a commit to KazydubB/drill that referenced this pull request Sep 25, 2019
…xpressions (apache#398)

* Fixed pushing istrue and isfalse conditions. Added unit tests to test boolean conditions.
@arina-ielchiieva

Copy link
Copy Markdown
Member

ashevchuk123 pushed a commit to mapr/incubator-drill that referenced this pull request Oct 28, 2025
…xpressions (apache#398)

* Fixed pushing istrue and isfalse conditions. Added unit tests to test boolean conditions.
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.

6 participants